From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 7D92726AD2 for ; Wed, 21 Aug 2019 06:02:42 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id iSloNyxuJEmV for ; Wed, 21 Aug 2019 06:02:42 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id CB14125F54 for ; Wed, 21 Aug 2019 06:02:41 -0400 (EDT) Date: Wed, 21 Aug 2019 13:02:22 +0300 From: Alexander Turenko Subject: [tarantool-patches] Re: [PATCH] Replace sendmsg with sendto shortcut Message-ID: <20190821100221.3pi5fo2vzb36q2cb@tkn_work_nb> References: <20190820120102.60158-1-m.melentiev@corp.mail.ru> <685c8df1-3ebb-07d7-44ba-2c79dce115c9@corp.mail.ru> <20190820233431.7ifg6jrvp4h7xmru@tkn_work_nb> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: Maxim Melentiev Cc: tarantool-patches@freelists.org, Konstantin Osipov , Kirill Yukhin Please, keep all participants in To / CC. CCed Kostja. CCed Kirill. WBR, Alexander Turenko. On Wed, Aug 21, 2019 at 11:23:40AM +0300, Maxim Melentiev wrote: > Pushed with changes to > https://github.com/tarantool/tarantool/tree/fix_sendmsg > > On 21/08/2019 02:34, Alexander Turenko wrote: > > This patch looks good to me. > > > > Minor comments are below. It does not require you to send the patch > > again. You can just answer here to comments and provide a branch with a > > new version of the patch. It is usual when a patch is not changed much. > > > > Kostya, are you have objections here? > > > > WBR, Alexander Turenko. > > > > > Replace sendmsg with sendto shortcut > > Maybe it worth to add 'systemd:' prefix to the commit header (and start > > a header with a small letter so). > > > > On Tue, Aug 20, 2019 at 03:16:35PM +0300, Maxim Melentiev wrote: > > > ping Konstantin Osipov, Alexander Turenko > > > > > > Sorry, forgot to CC in initial email and then pinged in wrong thread. > > > > > > On 20/08/2019 15:01, Max Melentiev (Redacted sender m.melentiev for DMARC) > > > wrote: > > > > There is a problem with calculating .msg_namelen field > > > > of msghdr struct. Instead of > > > > > > > > .msg_name = &sa, > > > > .msg_namelen = sizeof(sa.sun_family) + strlen(sd_unix_path), > > > > > > > > it must set as > > > > > > > > .msg_namelen = sizeof(sa) // larger value than current invalid one > > > > > > > > It works on linux but when I tried to enable this feature for macOS > > > > it didn't (maybe because of different order of fields in the struct). > > > > > > > > Instead of fixing calculation, I've replaced original sendmsg call > > > > with sendto, because it's a convenient shortcut which > > > > simplifies code and can prevent such mistakes. > > I suggest to add 'Part of #xxxx' or 'Needed for #xxxx'. > > > > > > --- > > > > > > > > Preview: https://github.com/printercu/tarantool/commit/bc30b1093be1d03507bf84ca218b03bec5e0244a > > > > > > > > src/systemd.c | 13 ++----------- > > > > 1 file changed, 2 insertions(+), 11 deletions(-) > > > > > > > > diff --git a/src/systemd.c b/src/systemd.c > > > > index b6c48afe2..9c0a26dd2 100644 > > > > --- a/src/systemd.c > > > > +++ b/src/systemd.c > > > > @@ -100,23 +100,14 @@ int systemd_notify(const char *message) { > > > > struct sockaddr_un sa = { > > > > .sun_family = AF_UNIX, > > > > }; > > > > - struct iovec vec = { > > > > - .iov_base = (char *)message, > > > > - .iov_len = (size_t )strlen(message) > > > > - }; > > > > - struct msghdr msg = { > > > > - .msg_iov = &vec, > > > > - .msg_iovlen = 1, > > > > - .msg_name = &sa, > > > > - }; > > > > > > > > - msg.msg_namelen = sizeof(sa.sun_family) + strlen(sd_unix_path); > > > > strncpy(sa.sun_path, sd_unix_path, sizeof(sa.sun_path)); > > > > if (sa.sun_path[0] == '@') > > > > sa.sun_path[0] = '\0'; > > > > > > > > say_debug("systemd: sending message '%s'", message); > > > > - ssize_t sent = sendmsg(systemd_fd, &msg, MSG_NOSIGNAL); > > > > + ssize_t sent = sendto(systemd_fd, message, (size_t)strlen(message), > > > > + MSG_NOSIGNAL, (struct sockaddr*)&sa, sizeof(sa)); > > Nit: As I see we usually split an asterics in a pointer type from a > > pointed type with a space and also split a type in parentheses from a > > value when casting. So: > > > > (size_t)strlen(message) -> (size_t) strlen(message) > > (struct sockaddr*)&sa -> (struct sockaddr *) &s > > > > > > if (sent == -1) { > > > > say_syserror("systemd: failed to send message"); > > > > return -1; > > > > -- > > > > 2.21.0 > > > > > > > > > > > > . > > > > > >