Re: [dpdk-dev] [PATCH 6/7] cmdline: support Windows

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

On Sun, 28 Jun 2020 23:23:11 -0700, Ranjit Menon wrote:
> On 6/28/2020 7:20 AM, Fady Bader wrote:
> > Hi Dmitry,
> > I'm trying to run test-pmd on Windows and I ran into this error with cmdline.
> >
> > The error log message is :
> > In file included from ../app/test-pmd/cmdline_flow.c:23:
> > ..\lib\librte_cmdline/cmdline_parse_num.h:24:2: error: 'INT64' redeclared as different kind of symbol
> >    INT64
> >
> > In file included from C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/winnt.h:150,
> >                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/minwindef.h:163,
> >                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/windef.h:8,
> >                   from C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/windows.h:69,
> >                   from ..\lib/librte_eal/windows/include/rte_windows.h:22,
> >                   from ..\lib/librte_eal/windows/include/pthread.h:20,
> >                   from ..\lib/librte_eal/include/rte_per_lcore.h:25,
> >                   from ..\lib/librte_eal/include/rte_errno.h:18,
> >                   from ..\lib\librte_ethdev/rte_ethdev.h:156,
> >                   from ../app/test-pmd/cmdline_flow.c:18:
> > C:/mingw-w64/x86_64/mingw64/x86_64-w64-mingw32/include/basetsd.h:32:44: note: previous declaration of 'INT64' was here
> >     __MINGW_EXTENSION typedef signed __int64 INT64,*PINT64;
> >
> > The same error is for the other types defined in cmdline_numtype.
> >
> > This problem with windows.h is popping in many places and some of them are
> > cmdline and test-pmd and librte_net.
> > We should find a way to exclude windows.h from the unneeded places, is there any
> > suggestions on how it can be done ?  
> 
> We ran into this same issue when working with the code that is on the 
> draft repo.
> 
> The issue is that UINT8, UINT16, INT32, INT64 etc. are reserved types in 
> Windows headers for integer types. We found that it is easier to change 
> the enum in cmdline_parse_num.h than try to play with the include order 
> of headers. AFAIK, the enums were only used to determine the type in a 
> series of switch() statements in librte_cmdline, so we simply renamed 
> the enums. Not sure, if that will be acceptable here.

+1 for renaming enum values. It's not a problem of librte_cmdline itself but a
problem of its consumption on Windows, however renaming enum values doesn't
break ABI and winn make librte_cmdline API "namespaced".

I don't see a clean way not to expose windows.h, because pthread.h depends on
it, and if we hide implementation, librte_eal would have to export pthread
symbols on Windows, which is a hack (or is it?).

-- 
Dmitry Kozlyuk