>> +//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. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
