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

Mon Dec 16 16:06:19 UTC 2013

On Fri, Dec 13, 2013 at 5:13 PM, Alin Serdean
<aserdean at cloudbasesolutions.com> wrote:
> This is an addition to the following patch:
> http://openvswitch.org/pipermail/dev/2013-November/034001.html .
> Beside the windefs.h creation more typedefs, defines and pragmas we're added.
>
> Signed-off-by: Alin Serdean <aserdean at cloudbasesolutions>
> ---
> ---
> diff --git a/Makefile.am b/Makefile.am
> index a6d985d..6d9899a 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -10,6 +10,11 @@ ACLOCAL_AMFLAGS = -I m4
>  SUBDIRS = datapath
>
>  AM_CPPFLAGS = $(SSL_CFLAGS)
> +
> +if WIN32
> +AM_CPPFLAGS += -I $(top_srcdir)/include/windows
> +endif
> +
>  AM_CPPFLAGS += -I $(top_srcdir)/include
>  AM_CPPFLAGS += -I $(top_srcdir)/lib
>  AM_CPPFLAGS += -I $(top_builddir)/lib
> diff --git a/include/windows/windefs.h b/include/windows/windefs.h
> new file mode 100644
> index 0000000..e19d0fa
> --- /dev/null
> +++ b/include/windows/windefs.h
> @@ -0,0 +1,105 @@
> +/*
> + * Copyright (c) 2013 Nicira, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef WINDEFS_H
> +#define WINDEFS_H 1
> +
> +#include <Winsock2.h>
> +#include <In6addr.h>
> +#include <WS2tcpip.h>
> +#include <windows.h>
> +
> +
> +//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?

> +#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.

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 ffs __lzcnt
> +#define inline __inline
> +#define strtok_r strtok_s
> +#define __func__ __FUNCTION__
> +#define snprintf _snprintf
> +#define strcasecmp _stricmp
> +#define strncasecmp _strnicmp
> +#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.
> +#define ssize_t int
I see some recommendations of using SSIZE_T from BaseTsd.h. Would that
make sense?
> +#define u_int8_t uint8_t
> +#define u_int16_t uint16_t
> +#define u_int32_t uint32_t
> +#define u_int64_t uint64_t
> +
> +typedef unsigned char __u8;
> +typedef unsigned short __u16;
> +typedef unsigned int  __u32;
> +typedef unsigned long long __u64;
> +typedef unsigned short __le16;
> +typedef unsigned short __be16;
> +typedef unsigned int __le32;
> +typedef unsigned int __be32;
> +typedef unsigned long long __le64;
> +typedef unsigned long long __be64;
> +typedef unsigned long int sigset_t;
> +
> +#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

> +
> +//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?

> +
> +#undef WCOREDUMP
> +#define WCOREDUMP(status) ((status) & 0x80)
> +#undef WEXITSTATUS
> +#define WEXITSTATUS(status) (((status)  & 0xff00) >> 8)
> +#undef WIFEXITED
> +#define WIFEXITED(status) (WTERMSIG(status) == 0)
> +#undef WIFSTOPPED
> +#define WIFSTOPPED(status) (((status) & 0xff) == 0x7f)
> +#undef WIFSIGNALED
> +#define WIFSIGNALED(status) (!WIFSTOPPED(status) && !WIFEXITED(status))
> +#undef WSTOPSIG
> +#define WSTOPSIG(status) WEXITSTATUS(status)
> +#undef WTERMSIG
> +#define WTERMSIG(status) ((status) & 0x7f)
> +#undef WCOREDUMP
> +#define WCOREDUMP(status) ((status) & 0x80)

> +
> +#endif /* windefs.h */
> --- a/m4/openvswitch.m4
> +++ b/m4/openvswitch.m4
> @@ -64,6 +64,9 @@ AC_DEFUN([OVS_CHECK_WIN32],
>     AM_CONDITIONAL([WIN32], [test "$WIN32" = yes])
>     if test "$WIN32" = yes; then
>        AC_DEFINE([WIN32], [1], [Define to 1 if building on WIN32.])
> +      AH_BOTTOM([#ifdef WIN32
> +#include "include/windows/windefs.h"
> +#endif])
>     fi])
>
>  dnl Checks for Netlink support.
> -----