[ovs-dev] [PATCH] V2 windefs: common include for MSVC

[Alin Serdean]

>> +//Disable warnings and runtime checks
>> +#pragma warning( disable : 4116 4090 4700 4005 4133 4028 4098 4293 4715 4047)
>> +#pragma runtime_checks( "", off )
>> +#pragma warning( disable : 4996 ) //deprecated functions
>> +#pragma warning( disable : 4244 ) //possible loss of data
>> +#pragma warning( disable : 4146 ) //unary minus operator applied to unsigned type, result still unsigned
>> +#pragma warning( disable : 4018 ) //'>=' : signed/unsigned mismatch
>
>Why are we disabling the warnings? Some of them look like may actually
>help with bugs. Worst case of not disabling them is that we will seem
>them during compilation right?
>
Majority of them are the same as the onex being passed in gcc (-Wno-sign-compare -Wunused-parameter etc. ) but I have no problem removing them it will just be a longer log to look at :).
Although this pragma: #pragma runtime_checks( "", off ) if not there will be runtime checks if there are local variables used but not set.
>
>> +#pragma comment(lib, "dbghelp.lib")
>> +#pragma comment(lib, "Advapi32.lib")
>> +#pragma comment (lib, "MsWSock.Lib")
>> +#pragma comment (lib, "WS2_32.Lib")
>
>Should we only add the above in a commit that includes code changes
>that needs any of these libraries. Otherwise, when any of the
>functions that use these libraries are actually removed in the future,
>we won't know to remove these libraries. For example, I do not know
>why we need the dbghelp.lib. But, if you add it to this file at the
>same time you make a change in code which needs this library, I will
>know.
>
Agreed. I will remove them when I send the new patch.
>I looked at doing this with autoconf's AC_SEARCH_LIBS, but it doesn't
>work for windows. We can do this with AC_LINK_IFELSE(), but I am not
>sure if it has any extra advantages compared to #pragma comment.
>
>> +#define caddr_t char*
>I see that caddr_t is only used in lib/netdev-linux.c. If we don't
>compile that file for Windows, we won't need the above #define.
I will remove it in the new patch.
>> +#define ssize_t int
>I see some recommendations of using SSIZE_T from BaseTsd.h. Would that
>make sense?
Totally makes sense and will change it in the new patch.
>
>> +#define LOG_EMERG   1
>> +#define LOG_ALERT   1
>> +#define LOG_CRIT    1
>> +#define LOG_ERR     4
>> +#define LOG_WARNING 5
>> +#define LOG_NOTICE  6
>> +#define LOG_INFO    6
>> +#define LOG_DEBUG   6
>> +#define LOG_NDELAY  6
>> +#define LOG_DAEMON  6
>
>Is there any reason for redefinition of the above a little differently
>than what we have for Linux? Why not use the
>http://en.wikipedia.org/wiki/Syslog#Severity_levels
>
You are totally right. What do you think of the following:
#define	LOG_EMERG	0	/* system is unusable */
#define	LOG_ALERT	1	/* action must be taken immediately */
#define	LOG_CRIT	        2	/* critical conditions */
#define	LOG_ERR		3	/* error conditions */
#define	LOG_WARNING	4	/* warning conditions */
#define	LOG_NOTICE	5	/* normal but significant condition */
#define	LOG_INFO	        6	/* informational */
#define	LOG_DEBUG	7	/* debug-level messages */
#define     LOG_NDELAY     8      /* don't delay open */
#define     LOG_DAEMON   24    /* system daemons */
>> +
>> +//Force undefines from config.h
>> +#undef HAVE_EXECINFO_H
>> +#undef VLOG_MODULE
>> +#undef HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC
>> +#undef HAVE_MNTENT_H
>> +#undef HAVE_SYS_STATVFS_H
>> +#undef HAVE_STATVFS
>> +#undef __CHECKER__
>> +#undef USE_LINKER_SECTIONS
>> +#undef DELETE
>> +#undef HAVE_BACKTRACE
>> +#undef HAVE_MALLOC_HOOKS
>I don't think we have to do the config.h undefs. Autoconf should take
>care and undef the above automatically.
>I also see VLOG_MODULE being undef'd. I see that on windows, autoconf
>will generate vlog-modules.def automatically and vlog logic works
>correctly. Don't you see that?
Sorry about that :), artefacts from my former port we can remove them.

If there are no further remarks I will send in the updated version.

Alin.