[ovs-dev] [PATCH 1/5] lib/util.c: add ovs_windows_only() cpp macro

[Nithin Raju]

On Oct 22, 2014, at 9:06 AM, Nithin Raju <nithin at vmware.com>
 wrote:

> 
> I'm not sure I'm happy about this.
> 
> For the only uses of this macro introduced in this patch series, which
> are assertions on variables that are always present, the code does not
> have to be Windows-only after the preprocessor.  One could simply write:
> 
>   #ifdef _WIN32
>   enum { WINDOWS = 1 };
>   #else
>   enum { WINDOWS = 0 };
>   #endif

Ben,
I was addressing your comments.

The code I have in dpif-netlink.c is in common code.

Eg.:
dpif_netlink_refresh_channels()
[...]
    ovs_windows_only(ovs_assert(n_handlers <= 1));
    ovs_windows_only(ovs_assert(dpif->n_handlers <= 1));

If we incorporate your comment, the code might look like this:
#ifdef _WIN32
enum {WINDOWS = 1}
#else
enum {WINDOWS = 0}
#endif

if (WINDOWS) {
    ovs_assert(n_handlers <= 1);
}

Did you mean this?

There are obviously other clever ways of writing this ASSERT:
    ovs_assert((WINDOWS && n_handlers <= 1) || !WINDOWS);

> and then add "WINDOWS &&" to the assertions.


Which one do you prefer? Did you mean that you prefer the latter approach? I am fine any way as long as we done have #ifdef/#endif _WIN32.

If I am missing something obvious, pls. feel free to point it out.

Thanks,
-- Nithin