[ofa-general] Re: [ib-diag] sminfo: add support for WinOF

[Sasha Khapyorsky]

On 10:40 Sat 14 Feb     , Sean Hefty wrote:
> >> Would there be any objection to including the windows source files (.c and
> >.h)
> >> in the mgmt tree?
> >
> >Which files? Basically I prefer to not have unrelated things in my tree,
> >but let's see specific needs.
> 
> So far, I have windows/ibdiag_osd.h, ibdiag_windows.c, and
> windows/cl_nodenamemap.h.

Isn't cl_nodenamemap.h part of complib?

> 
> My goal is to have the ib-diags support both Windows and Linux, so Windows files
> are related in that respect.  Making an exception for the build files is
> reasonable IMO, given the WinOF build environment.
> 
> >> diff --git a/infiniband-diags/src/ibdiag_common.c b/infiniband-
> >diags/src/ibdiag_common.c
> >> index bda1efa..154e00c 100644
> >> --- a/infiniband-diags/src/ibdiag_common.c
> >> +++ b/infiniband-diags/src/ibdiag_common.c
> >> @@ -43,15 +43,14 @@
> >>  #include <stdlib.h>
> >>  #include <stdarg.h>
> >>  #include <sys/types.h>
> >> -#include <unistd.h>
> >>  #include <ctype.h>
> >> -#include <config.h>
> >>  #include <getopt.h>
> >>
> >>  #include <infiniband/umad.h>
> >>  #include <infiniband/mad.h>
> >>  #include <ibdiag_common.h>
> >>  #include <ibdiag_version.h>
> >> +#include "ibdiag_osd.h"
> >
> >Wouldn't it be easier (at least for linux developers :)) instead
> >of filtering out pretty standard header files to put such files under
> >winof tree? (Including config.h, this file is generated by autotools,
> >as far as I could see it is not used in WinOF, so it should be easy to
> >keep this as "osd" file).
> 
> unistd.h is an 'osd' type file, so I think it makes more sense to isolate it to
> an osd related area.  But if you really prefer, I can abstract these.  (Windows
> provides an errno.h file, so at least there's some precedence.)
> 
> >> @@ -273,7 +272,7 @@ int ibdiag_process_opts(int argc, char * const argv[],
> >void *cxt,
> >>       char str_opts[1024];
> >>       const struct ibdiag_opt *o;
> >>
> >> -     memset(opts_map, 0, sizeof(opts_map));
> >> +     memset((void *) opts_map, 0, sizeof(opts_map));
> >
> >Hmm, why is this casting needed?
> 
> opts_map is declared as const - (i.e. my compiler whined at me)

Probably it is reasonable to just drop const then. I don't see what this
const really does.

Sasha

> 
> >> -int main(int argc, char **argv)
> >> +int CDECL main(int argc, char **argv)
> >
> >Would compiler flag /Gd do the same without code modification?
> >
> >(http://msdn.microsoft.com/en-us/library/46t77ak2(VS.71).aspx)
> 
> I'll see if I can get this to work.  My quick test gave me compiler option
> conflicts, so I'll have to look into this more. 
> 
> - Sean
>