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

Sean Hefty sean.hefty at intel.com
Sat Feb 14 10:40:57 PST 2009


>> 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.

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)

>> -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




More information about the general mailing list