[PATCH] Unix datagram socket messaging

Stefan (metze) Metzmacher metze at samba.org
Wed Apr 16 04:00:04 MDT 2014


Hi Volker,

> https://git.samba.org/?p=vl/samba.git/.git;a=shortlog;h=refs/heads/unix-msg
> 
> contains the result of Jeremy's preliminary review of the
> unix datagram based messaging code for source3.
> 
> This is the request for formal review and push to master.

The notify patches seem to be independent, can you move them to be the
first patches?

The same applies to "printing_cups: Call the msg_ctx destructor on exit"
and "smbd: Call the msg_ctx destructor for background jobs".

iov_buf* can also be added first and then used in the unix_msg code
instead of the static copy.

They all have "Reviewed-by: Stefan Metzmacher <metze at samba.org>"

I haven't fully reviewed the real changes yet, but have some initial
comments:

Can we pass tevent_context to poll_funcs_init_tevent() instead of
using: func.private_data = ev; in the caller?

Are we sure we don't get problems when having all messaging sockets in
just one subdirectory?

There're environments with just winbindd running, so we might need a
periodic
cleanup run not only in smbd.

In
https://git.samba.org/?p=vl/samba.git/.git;a=commitdiff;h=777954434bb7404659fbb7f519a4069b1100c400
we have

+       struct sockaddr_un addr = { 0, };

...

+               ret = bind(ctx->sock, (struct sockaddr *)&addr,
sizeof(addr));

I think we should cast with (struct sockaddr *)(void *)&addr, in order
to avoid
strict aliasing warnings on some systems.

After socket() we should set FD_CLOEXEC and O_NONBLOCK similar to
tsocket_bsd_common_prepare_fd() or set_blocking() and
smb_set_close_on_exec().
MSG_DONTWAIT might not be available everywhere.


+ if (errno != EWOULDBLOCK) {

may need some more magic and also check for EAGAIN,
see tsocket_bsd_error_from_errno() or commit
f5a3d74264abb25009e73b1b35d62db34fa62343.
... ok, I just found that it's fixed in
https://git.samba.org/?p=vl/samba.git/.git;a=commitdiff;h=325e62cbb116abdec7cf1466862bd12ecd42224e
I think we should squash that and also this one
https://git.samba.org/?p=vl/samba.git/.git;a=commitdiff;h=d44c82a1edddb0d838702e15e00538faae9386bc

+ unlink(ctx->path);

We may need to check that ctx->path is not an empty string?

metze


More information about the samba-technical mailing list