[ovs-dev] [PATCH v2] windows: wmi add include

[Alin Serdean]

> -----Original Message-----
> From: Ben Pfaff [mailto:blp at ovn.org]
> Sent: Wednesday, February 1, 2017 7:36 PM
> To: Alin Serdean <aserdean at cloudbasesolutions.com>
> Cc: Guru Shetty <guru at ovn.org>; Sairam Venugopal
> <vsairam at vmware.com>; dev at openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v2] windows: wmi add include
> 
> On Wed, Feb 01, 2017 at 12:56:44AM +0000, Alin Serdean wrote:
> >
> > > -----Original Message-----
> > > From: Ben Pfaff [mailto:blp at ovn.org]
> > > Sent: Tuesday, January 31, 2017 11:35 PM
> > > To: Alin Serdean <aserdean at cloudbasesolutions.com>
> > > Cc: dev at openvswitch.org
> > > Subject: Re: [ovs-dev] [PATCH v2] windows: wmi add include
> > >
> > > On Mon, Jan 30, 2017 at 07:42:31AM +0000, Alin Serdean wrote:
> > > > Add 'util.h' to includes otherwise the result of the function
> > > > 'ovs_format_message' will be unknown and be converted to int,
> > > > triggering an abort of vswitchd.
> > >
> > > Is there a way to make MSVC fail the compile if a function is called
> > > without a visible prototype?  That would prevent this problem.
> > >
> > > Alternatively, if it's possible to get a warning but not an error,
> > > it might be possible to add something to the appveyor build script
> > > to fail that build if any such warning is reported.
> > [Alin Serdean] Thanks for the idea Ben(made me facepalm for not thinking
> of it).
> > There is something like -Wimplicit-function-declaration(-Wall) and I could
> issue an error while compiling.
> 
> Sounds good.
> 
> > I tried on Linux to add a bogus function, but make stopped during linking
> (from what I remember -Werror is needed to stop compiling).
> 
> By default, on Linux, we enable a lot of warnings but we do not make the
> compiler turn them into errors, because we assume that the common case is
> that some end user or distributor is building a known-good version of OVS
> and that they don't want the build to stop because they're using a slightly
> different version of the compiler that issuse warnings in slightly different
> circumstances.
[Alin Serdean] Good point. I think on Windows the majority will use msi's for ease of use, but that is another debate
> 
> But we encourage developers to configure with --enable-Werror, which
> turns all warnings into errors (that break the build).  That way, developers
> can't miss warnings that might be about important issues.
> 
> I thought that we used --enable-Werror in the travis CI system too, but now I
> see that I'm mistaken.  Maybe we should enable it there.
[Alin Serdean] I have no idea atm, I will check it out.
But even if it is enable, for sure it won't work because:
https://github.com/openvswitch/ovs/blob/master/build-aux/cccl#L148
That is the wrapper that transform the gcc arguments to cl(visual studio compiler) arguments.
I will spend time to try to come up with a list of matches between the two type of compiler arguments.
At the moment we get level one warnings from the build, because that is the default setting. I.E.:
https://ci.appveyor.com/project/blp/ovs/build/1.0.2752#L491
I will create a test appveyor account to set up the full scheme, this is too important to ignore any further.
Thanks a lot for the idea and answering my questions!
> 
> > Since this has deep implication is it ok if I stop compiling for this type of
> issue?
> 
> I think so, at least for this particular warning.
> 
> > I enabled it locally and fixing current issues that we have in the code(most
> of them are ok since the type are int, but we need to fix them).