On Fri, Jul 11, 2014 at 01:06:50AM +0000, Alin Serdean wrote:
> Adding CreateFile, WriteFile, ReadFile and CloseHandle instead of linux
> counterparts when compiling netlink-socket.c with MSVC. Also add socket pid
> generation function that will later on will be needed in the kernel
> extension.
>
> Signed-off-by: Alin Gabriel Serdean <[email protected]>
I couldn't get this to apply, I guess the recent changes to this file
caused conflicts.
This has style issues.
Why does set_sock_pid_in_kernel() sleep?
Why does set_sock_pid_in_kernel() define a local 1-element struct
type?
It looks like set_sock_pid_in_kernel() is not thread safe.
nl_sock_create() checks for CreateFileA() failure much later than the
Linux version does for socket() failure. Why?
nl_sock_create() uses errno to retrieve errors. I'm pretty sure that
Winsock needs sock_errno() instead, do these Windows functions also?
It looks like nl_sock_create() is just guessing an arbitrary buffer
size on Windows. It should get the correct value.
It looks like every reference to an nl_sock fd is cast to HANDLE on
Windows. I guess that you should declare a HANDLE member on windows
instead of an int fd.
This doesn't make any sense, did you mean to delete the line that just
says LPOVERLAPPED?
> + if (RecvOverlapped.hEvent == NULL) {
> + LPOVERLAPPED
> + retval = -1;
> + }
> + retval = WriteFile((HANDLE)sock->fd, ofpbuf_data(msg),
> ofpbuf_size(msg),
> + NULL, NULL);
> +#endif
The definition of nl_sock_drain() contains an inline copy of
drain_rcvbuf(), which is in turn #if'd out on Windows. It would make
more sense not to #if the latter out and just to call it from
nl_sock_drain().
This is a weird change:
> if (!family_attrs[CTRL_ATTR_MCAST_GROUPS]) {
> +#ifndef _WIN32
> error = EPROTO;
> +#else
> +#define OVS_VPORT_MCGROUP_FALLBACK_ID 33
> + *multicast_group = 33;
> + VLOG_WARN("%s-%s: has no multicast group, using fallback %d",
> + family_name, group_name, *multicast_group);
> + error = 0;
> +#endif
> goto exit;
> }
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
