[ovs-dev] [PATCH v2] windows/lib: Fix Windows C++ compilation issues on common headers

Ben Pfaff blp at ovn.org
Thu Nov 30 18:03:38 UTC 2017


On Thu, Nov 30, 2017 at 02:47:34PM +0530, Shashank Ram wrote:
> On Thu, Nov 30, 2017 at 11:45 AM, Ben Pfaff <blp at ovn.org> wrote:
> 
> > On Wed, Nov 29, 2017 at 05:13:14PM -0800, Sairam Venugopal wrote:
> > > Found when compiling the code with C++ binaries. Most of the issues are
> > > due to missing explicit cast.
> > >
> > > Signed-off-by: Sairam Venugopal <vsairam at vmware.com>
> > > Signed-off-by: Shireesh Kumar Singh <shireeshkum at vmware.com>
> > > Co-authored-by: Shireesh Kumar Singh <shireeshkum at vmware.com>
> >
> > Hi Sairam, thanks for passing this along.
> >
> > The commit message does not explain the problem with
> > PADDED_MEMBERS_CACHELINE_MARKER.  Can you help me to understand it?  The
> > reason I would expect to add a new MSVC-only version is that it is
> > impossible to write a single version (for C) that works with MSVC, GCC,
> > and Clang, but I don't yet know a reason that is true.
> >
> > The header files in lib/ aren't exported for public consumption, which
> > means that software other than Open vSwitch itself isn't expected to use
> > them.  Of course, VMware and possibly others are actually doing that.  I
> > have mixed feelings about that.  It's nice to know that OVS has useful
> > header files, but I'm not so excited about the idea that every OVS
> > header file has to work OK in C++.  Do you have any thoughts on how an
> > OVS developer can guess whether a given header file will be used in C++?
> 
> 
> Sai and Shireesh, thanks for the patch. I share the same sentiments as Ben
> regarding
> the usage of the header files in lib/. Since OVS is written in C, and this
> patch attempts
> to allow some headers in lib/ to be compiled using MSVC++, it might set a
> precedent
> where other headers might require to be changed as well just to be able to
> compile
> it in C++.  This is not very intuitive to an OVS developer about the
> consumption
> of the header file by a C++ compiler.
> I feel it would be better to add a public header file for some of the
> common headers
> being consumed outside of OVS, and maybe even a version of the header file
> that compiles fine in C++.

To add a few more details, OVS already has a policy that headers in
include/openvswitch should compile as C++, and it even has checks that
makes sure that they do.  So, if we decide to move some of the "lib"
headers to "include/openvswitch", they would naturally work in C++, but
on the other hand I'd like to have some idea of which headers are
important for that purpose and ideally they would fall into some easily
understood category or categories.


More information about the dev mailing list