From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from atuin.qyliss.net (localhost [IPv6:::1]) by atuin.qyliss.net (Postfix) with ESMTP id B665D3004; Tue, 28 Oct 2025 15:39:08 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id 97A232FF3; Tue, 28 Oct 2025 15:39:05 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-26) on atuin.qyliss.net X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,DMARC_MISSING,RCVD_IN_DNSWL_LOW,SPF_HELO_PASS autolearn=unavailable autolearn_force=no version=4.0.1 Received: from fout-a6-smtp.messagingengine.com (fout-a6-smtp.messagingengine.com [103.168.172.149]) by atuin.qyliss.net (Postfix) with ESMTPS id A41F52FF1 for ; Tue, 28 Oct 2025 15:39:03 +0000 (UTC) Received: from phl-compute-05.internal (phl-compute-05.internal [10.202.2.45]) by mailfout.phl.internal (Postfix) with ESMTP id 2FBD5EC0572; Tue, 28 Oct 2025 11:39:02 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-05.internal (MEProxy); Tue, 28 Oct 2025 11:39:02 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alyssa.is; h=cc :cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm2; t=1761665942; x=1761752342; bh=FVwEG7YxyW WYry4YoUJQlkEM8HYKkWSZc3ZiejwdI+o=; b=gV4ubyyq10I6zUJxojkXuYidHX aUyihURD5DxPNXQgV3apk5/DoaweKxpHhO0sph7oiaoYLyODvAd1mQT1NW/p4opi Mc8/5Lel9FXUBz9U+0pTukc6w9F6WKVm9lxEmoBxD+DFz9wQIvdjKAYti55kd9M6 RHjDbQuxTlLAY/o5/qZp3WH3FSJVbkV+lpXmKuz/e1eMVWkRBcfgNPN7RBjBkhfT lrB62LOtM9qmoL0zQgm1uXPcIPUYcriki4E3R/6SdoIeO7z8UqB6S+Cd/VlK0evr b5mX7wDNTVT3PtC4o6IEYbHyt5zBsP64f8pmDG1BREAkpq8iOzWE6Ld7vC5w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t= 1761665942; x=1761752342; bh=FVwEG7YxyWWYry4YoUJQlkEM8HYKkWSZc3Z iejwdI+o=; b=Y/kxmYtpjNdG3ebpeImkOGJ4HDLsz0nKUuCj0Kyd4zBkaUmbsMD xsg6pYV2jUUTpGYwMxSveTPfN9JeZNL+S/nevBp5mzgiTtWC1swkxLMHAkKvkvYo ZdErqY2ULQ94rr44d9baUMKhmDReBmU15zoAEtKglhE1D1jihVxWli/PDa+rsHz9 imwgnESt/fUtEF7aUADwbe90HBd1PV1lbB333VquqNTj4f2LNItr25YMB4wOxsmd FiEp0zOzG4Wp3jWQ51JYY0KOpEoL9su6MiHE6VOjsbzr347vsEfqqL8fFerIvg2y 9nOpzMyy9eEMC6SXIalJJoWNp+1QEP/m8lQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdeggdduieduvdefucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu rghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujf gurhephffvvefujghffffkgggtsehgtderredttdejnecuhfhrohhmpeetlhihshhsrgcu tfhoshhsuceohhhisegrlhihshhsrgdrihhsqeenucggtffrrghtthgvrhhnpeetheevud fgjefghefhieejudelkeeljeegvdekueeuhffhgedvveefteevgeetieenucevlhhushht vghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehhihesrghlhihsshgrrd hishdpnhgspghrtghpthhtohepvddpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtohep uggvmhhiohgsvghnohhurhesghhmrghilhdrtghomhdprhgtphhtthhopeguvghvvghlse hsphgvtghtrhhumhdqohhsrdhorhhg X-ME-Proxy: Feedback-ID: i12284293:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 28 Oct 2025 11:39:01 -0400 (EDT) Received: by fw12.qyliss.net (Postfix, from userid 1000) id 49DBF4F8764; Tue, 28 Oct 2025 16:38:50 +0100 (CET) From: Alyssa Ross To: Demi Marie Obenour Subject: Re: [PATCH v4 1/2] tools: Add adapter tool for services using sd_notify In-Reply-To: <20251003-udev-v4-1-7d7344b14d11@gmail.com> References: <20251003-udev-v4-0-7d7344b14d11@gmail.com> <20251003-udev-v4-1-7d7344b14d11@gmail.com> Date: Tue, 28 Oct 2025 16:38:48 +0100 Message-ID: <87ms5b3uk7.fsf@alyssa.is> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Message-ID-Hash: QMRTGE4GHK6SJSCRVZAONRCMGEWMAHQI X-Message-ID-Hash: QMRTGE4GHK6SJSCRVZAONRCMGEWMAHQI X-MailFrom: hi@alyssa.is X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-devel.spectrum-os.org-0; header-match-devel.spectrum-os.org-1; header-match-devel.spectrum-os.org-2; header-match-devel.spectrum-os.org-3; header-match-devel.spectrum-os.org-4; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Spectrum OS Development X-Mailman-Version: 3.3.9 Precedence: list List-Id: Patches and low-level development discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Demi Marie Obenour writes: > This adapts programs using sd_notify for use with s6 readiness > notification. stdin and stdout are hard-coded for simplicity. > > Signed-off-by: Demi Marie Obenour > --- > systemd readiness notification has two > strict advantages over the s6 version: > > 1. It allows reliable reloading. > 2. It allows providing a status message that the service manager > can show in status output. > > s6 would actually benefit from both of these features. > --- > Changes since v1: > > - Hard-code file descriptors. > - Run wrapper as background process. > - Massively reduce code size. > - Use // instead of /* */ for comments. > - Check that the notification FD is a pipe and that the listening socket > is a socket. > - Rely on s6-ipc-socketbinder to create the listening socket. > - Do not unlink the listening socket. > --- > tools/default.nix | 1 + > tools/meson.build | 1 + > tools/sd-notify-adapter/meson.build | 4 + > tools/sd-notify-adapter/sd-notify-adapter.c | 114 ++++++++++++++++++++++= ++++++ > 4 files changed, 120 insertions(+) Looks correct, so just some convention/readability things. If you're happy with all my comments I can just change them all myself if you prefer not to send a new version of the patch =E2=80=94 up to you. > diff --git a/tools/sd-notify-adapter/meson.build b/tools/sd-notify-adapte= r/meson.build > new file mode 100644 > index 0000000000000000000000000000000000000000..6032a3a7704d49cae0655b43d= 0189444d3b15e4d > --- /dev/null > +++ b/tools/sd-notify-adapter/meson.build > @@ -0,0 +1,4 @@ > +# SPDX-License-Identifier: ISC > +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour > + > +executable('sd-notify-adapter', 'sd-notify-adapter.c', install: true) > diff --git a/tools/sd-notify-adapter/sd-notify-adapter.c b/tools/sd-notif= y-adapter/sd-notify-adapter.c > new file mode 100644 > index 0000000000000000000000000000000000000000..10f4e05eb602491540a792c7f= b5620d66d5bb989 > --- /dev/null > +++ b/tools/sd-notify-adapter/sd-notify-adapter.c > @@ -0,0 +1,114 @@ > +// SPDX-License-Identifier: MIT > +// SPDX-FileCopyrightText: 2025 Demi Marie Obenour > + > +#define _GNU_SOURCE 1 Like I said last time, this should be set by the build system. > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define ARRAY_SIZE(s) (sizeof(s)/sizeof(s[0])) > + > +enum { > + socket_fd, > + notification_fd, > +}; > + > +#define READY "READY=3D1" > +#define READY_SIZE (sizeof(READY) - 1) > + > +static void process_notification(struct iovec *const msg) > +{ > + ssize_t first_recv_size =3D recv(socket_fd, msg->iov_base, msg->iov_len, > + MSG_DONTWAIT | MSG_TRUNC | MSG_PEEK); I guess it just doesn't matter either way, but my question about why MSG_DONTWAIT from last time wasn't answered either. > + if (first_recv_size =3D=3D -1) { > + if (errno =3D=3D EINTR) > + return; // signal caught > + if (errno =3D=3D EAGAIN || errno =3D=3D EWOULDBLOCK) > + return; // spurious wakeup The check for these from the second recv was removed. Should this check also be removed? Is returning EAGAIN or EWOULDBLOCK here a valid thing for the kernel to do? > + err(EXIT_FAILURE, "recv from notification socket"); > + } > + assert(first_recv_size >=3D 0); Worth even checking? Would be a serious contract violation that would break all sorts of things. > + size_t size =3D (size_t)first_recv_size; > + if (size =3D=3D 0) > + return; // avoid arithmetic on NULL pointer > + if (size > msg->iov_len) { > + msg->iov_base =3D realloc(msg->iov_base, size); > + if (msg->iov_base =3D=3D NULL) > + err(EXIT_FAILURE, "allocation failure"); > + msg->iov_len =3D size; > + } > + ssize_t second_recv_size =3D recv(socket_fd, msg->iov_base, msg->iov_le= n, > + MSG_CMSG_CLOEXEC | MSG_TRUNC); > + if (second_recv_size =3D=3D -1) { > + if (errno =3D=3D EINTR) > + return; > + err(EXIT_FAILURE, "recv from notification socket"); > + } > + assert(first_recv_size =3D=3D second_recv_size); > + for (char *next, *cursor =3D msg->iov_base, *end =3D cursor + size; > + cursor !=3D NULL; cursor =3D (next =3D=3D NULL ? NULL : next + 1))= { > + next =3D memchr(cursor, '\n', (size_t)(end - cursor)); > + size_t message_size =3D (size_t)((next =3D=3D NULL ? end : next) - cur= sor); > + if (message_size =3D=3D READY_SIZE && > + memcmp(cursor, READY, READY_SIZE) =3D=3D 0) { > + ssize_t write_size =3D write(notification_fd, "\n", 1); > + if (write_size !=3D 1) > + err(EXIT_FAILURE, "writing to notification descriptor"); > + exit(0); > + } > + } > +} > + > +int main(int argc, char **) > +{ > + if (argc !=3D 1) > + errx(EXIT_FAILURE, "stdin is listening socket, stdout is notification = pipe"); > + // Main event loop. > + struct iovec v =3D { > + .iov_base =3D NULL, > + .iov_len =3D 0, > + }; It might be clearer for this to be a static, rather than a variable in main that's only ever used by a function it calls. It took me a while to figure out that it's being reused between calls (for realloc). And then you don't need the initializer either. :) --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQQGoGac7QfI+H5ZtFCZddwkt31pFQUCaQDjiAAKCRCZddwkt31p FWVsAQDZp7H9A3PlCNkigCl7ub0yO9mhwjzZfYr1zhzhjyuufwEAk45ka8QU2viF 0+jMfkroSLtRTKQkqtFFp2FIAA/Djww= =VLKg -----END PGP SIGNATURE----- --=-=-=--