Adding [email protected] 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 <[email protected]> > To: Sean Hefty <[email protected]> > CC: [email protected] > References: <[email protected]> > > 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 > [email protected] > http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general > > To unsubscribe, please visit http://openib.org/mailman/listinfo/openib- > general
_______________________________________________ general mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
