[ofw] RE: [Fwd: [ofa-general] Re: porting IB management code to Windows]

[Ishai Rabinovitz]

Adding ofw at lists.openfabrics.org to the discussion.

> -------- Original Message --------
> Subject: [ofa-general] Re: porting IB management code to Windows
> Date: Sat, 13 Dec 2008 22:30:14 +0200
> From: Sasha Khapyorsky <sashak at voltaire.com>
> To: Sean Hefty <sean.hefty at intel.com>
> CC: general at lists.openfabrics.org
> References: <000201c95bc5$510162a0$1e58180a at amr.corp.intel.com>
> 
> Hi Sean,
> 
> On 11:18 Thu 11 Dec     , Sean Hefty wrote:
> >
> > We've started porting the IB management code (IB-diags at this
point)
> to
> > Windows.  My strong preference is to avoid branching the code and
> instead keep a
> > single source code tree.  Is there any objection to accepting
changes
> against
> > the management tree to allow the code to run on both Linux and
> Windows?
> 
> Basically I have no objections against porting changes. And I also
> would
> prefer to keep a single code base.
> 
> However, I would prefer to minimize amount of needed changes and would
> really prefer to not get a lot of limitations in using modern C. I
will
> comment inline in the patch example below.
> 
> > (We can
> > figure out the logistics of build related files later.  I'm most
> concerned about
> > the code itself.)
> >
> > The patch below gives an example of the changes needed to make this
> happen.
> > Most are a result of compiler differences.
> >
> > - Sean
> >
> > --- infiniband-diags-1.4.2\src\sminfo.c	2008-10-19
11:34:42.000000000
> -0700
> > +++ scm\winof\branches\winverbs\tools\infiniband_diags\src\sminfo.c
> > 2008-12-10 15:06:01.096000000 -0800
> > @@ -37,12 +37,19 @@
> >
> >  #include <stdio.h>
> >  #include <stdlib.h>
> > +
> > +#if defined(_WIN32) || defined(_WIN64)
> > +#include <windows.h>
> > +#include <winsock2.h>
> > +#include "..\..\..\..\etc\user\getopt.c"
> > +#include "..\ibdiag_common.c"
> > +#else
> >  #include <unistd.h>
> >  #include <stdarg.h>
> >  #include <inttypes.h>
> >  #include <getopt.h>
> > +#endif
> 
> Could such ugly header mess be eliminated?
> 
> I'm not familiar with windows environment, but would expect that
> headers
> like <stdarg.h> exist there (although I may be wrong about it). Of
> course some header file may be missing, this is not so bad - you could
> add one somewhere under WinOF tree in the include path, then something
> like:
> 
> winof/include/path/getopt.h:
> 
> #ifndef WINOF_GETOPT_H
> #define WINOF_GETOPT_H
> 
> #include "..\..\..\..\etc\user\getopt.c"
> 
> #endif
> 
> could resolve the problem. And similar with another header files (also
> AFAIK WinOF is not using autotools, so file config.h could be also
good
> place for various wrappers).
> 
> > -#include <infiniband/common.h>
> >  #include <infiniband/umad.h>
> >  #include <infiniband/mad.h>
> >
> > @@ -72,13 +79,13 @@ enum {
> >  };
> >
> >  char *statestr[] = {
> > -	[SMINFO_NOTACT] "SMINFO_NOTACT",
> > -	[SMINFO_DISCOVER] "SMINFO_DISCOVER",
> > -	[SMINFO_STANDBY] "SMINFO_STANDBY",
> > -	[SMINFO_MASTER] "SMINFO_MASTER",
> > +	"SMINFO_NOTACT",
> > +	"SMINFO_DISCOVER",
> > +	"SMINFO_STANDBY",
> > +	"SMINFO_MASTER",
> >  };
> 
> Could VC++ understand C99 like initializations (maybe with using some
> flags)? I would really prefer to use something like this.
> 
> >
> > -#define STATESTR(s)	(((uint)(s)) < SMINFO_STATE_LAST ?
statestr[s]
> : "???")
> > +#define STATESTR(s)	(((unsigned int)(s)) < SMINFO_STATE_LAST
?
> statestr[s] :
> > "???")
> >
> >  int
> >  main(int argc, char **argv)
> > @@ -88,7 +95,7 @@ main(int argc, char **argv)
> >  	ib_portid_t portid = {0};
> >  	int timeout = 0;	/* use default */
> >  	uint8_t *p;
> > -	uint act = 0;
> > +	unsigned int act = 0;
> 
> All 'uint' -> 'unsigned int' conversions seem fine for me (I think we
> need to do this even w/out connection to WinOF porting issue).
> 
> >  	int prio = 0, state = SMINFO_STANDBY;
> >  	uint64_t guid = 0, key = 0;
> >  	extern int ibdebug;
> > @@ -97,8 +104,8 @@ main(int argc, char **argv)
> >  	char *ca = 0;
> >  	int ca_port = 0;
> >
> > -	static char const str_opts[] = "C:P:t:s:p:a:deDGVhu";
> > -	static const struct option long_opts[] = {
> > +	static char str_opts[] = "C:P:t:s:p:a:deDGVhu";
> > +	static struct option long_opts[] = {
> 
> I saw in your another email that 'const' issue could be solved (worst
> case it could be masked in WinOF config.h - #define const ). Right?
> 
> >  		{ "C", 1, 0, 'C'},
> >  		{ "P", 1, 0, 'P'},
> >  		{ "debug", 0, 0, 'd'},
> > @@ -112,7 +119,7 @@ main(int argc, char **argv)
> >  		{ "timeout", 1, 0, 't'},
> >  		{ "help", 0, 0, 'h'},
> >  		{ "usage", 0, 0, 'u'},
> > -		{ }
> > +		{ 0 }
> 
> Could VC be learned with some flags to understand {}? Basically we
> could
> except such change, but it will be hard to remember to follow this
rule
> on
> linux side :)
> 
> >  	};
> >
> >  	argv0 = argv[0];
> > @@ -188,7 +195,7 @@ main(int argc, char **argv)
> >
> >  	if (mod) {
> >  		if (!(p = smp_set(sminfo, &portid, IB_ATTR_SMINFO, mod,
> > timeout)))
> > -			IBERROR("query");
> > +			IBERROR("set");
> 
> This is fine (and guess is not related to porting issue :))
> 
> Sasha
> 
> >  	} else
> >  		if (!(p = smp_query(sminfo, &portid, IB_ATTR_SMINFO, 0,
> > timeout)))
> >  			IBERROR("query");
> >
> >
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
> 
> To unsubscribe, please visit
http://openib.org/mailman/listinfo/openib-
> general