From nobody Fri Feb 16 19:45:14 2024 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Tc2Tg6Lywz56Rtx for ; Fri, 16 Feb 2024 19:45:27 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-ed1-x535.google.com (mail-ed1-x535.google.com [IPv6:2a00:1450:4864:20::535]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Tc2Tg0NdXz4H6L for ; Fri, 16 Feb 2024 19:45:27 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-ed1-x535.google.com with SMTP id 4fb4d7f45d1cf-5640fef9fa6so621568a12.0 for ; Fri, 16 Feb 2024 11:45:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20230601.gappssmtp.com; s=20230601; t=1708112724; x=1708717524; darn=freebsd.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=rh2WqiG0+XRxdOIKmO/bJyrwo0vBbVBU0iAspO7PaSg=; b=fv637LYFc5NL8pT8OuOwmnl2sSVyrHMZW7f0fWF0xXgBdCLSwcBKPcVFCXg2t8N0e5 PuCWcCbwKDb5EJXM3cRy7aSm3hVlM8r8Yvnag6vwP+V/jo4ejpnpsnUn5zf2JFSVBvOG KB/RNwojhM8NIYvxnbzSXxI4quP9M5El9DrnyCzo5KhH/GBUg5+Pgc1ZYltZwgXsuur8 oDIDJkeHZcX7XTSoEyYxPlDYtegMnAuKX2f9xwiyXzyygIVuiXSp0KWeh0pQSBLBlgwJ Y8T/WU/QLsn2/Ch3JEps3+Er1M7TC5mPVdRxHe7CMk1l0S3A/HFUZJIL2z0r/6ntmOOB S2HQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708112724; x=1708717524; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=rh2WqiG0+XRxdOIKmO/bJyrwo0vBbVBU0iAspO7PaSg=; b=cN34dmUQinZmGIcPX7u030GNkZdX256LSCVFlnIVkP37XJGsSWmxZv8SroTwfXGOsX FT4EZBoNQ1YxSvDzhT0IeLBnHGx/uR4ZEk0bmgMRyuBQxld0EmBRGxUiyHo7jCL9Hzjx oCRAxEqOSHYtrm117H389JPshJUZi7u9QDqgEDklDza5sV6zA8OGcEmWfuQq4RJW49GY +stxVHmQJr2YfLnfn4qCCIoAPbImH5+JuwEV1EEiGLoZtigqcaqSmW+lk7Q5Ej15P7tm CjSDISPAo3u0vViHX3G7IQeegfUZuPPd5Om5gsU6vkcCCp1Nb0xD0Lw4ZljJGKdkTvZ7 JzeQ== X-Forwarded-Encrypted: i=1; AJvYcCV1oNjAUC5kuGF14SYBoXnFMJ2jFhnYpFbEvqZVqbEVGyW2uibVLaBfK5mD/sb6EkqClSElK4gBovSvKADXKJTWpZJEU+Dkt7wsvmxIb7ieNA== X-Gm-Message-State: AOJu0YwaY5JmDjd4f9KkhoMY6kPfnU1gsvRFESH/jMwZgub/AX1RDxPb OsQefV6doYV6DetkJ8qD3CQf+f0/h02vnGRY1aQWi3Wa7Zo3IZ/SfVrzUs9OdKC64u41KBrxTXH qq/z2olnccSCI69tPL6p1EymHfS48zQDhqcFCXw== X-Google-Smtp-Source: AGHT+IHpns15Q8AmvB2pKhQ3Xd0QYB2TRSfJp7g9j8EptzNWd5CaSgpoUQsnHAY8GI6afHHi0oqrMIuTpez3UoQaDgc= X-Received: by 2002:a05:6402:ca3:b0:563:d32f:544a with SMTP id cn3-20020a0564020ca300b00563d32f544amr3040674edb.1.1708112724271; Fri, 16 Feb 2024 11:45:24 -0800 (PST) List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 References: <202402160400.41G40Jgd018717@gitrepo.freebsd.org> In-Reply-To: From: Warner Losh Date: Fri, 16 Feb 2024 12:45:14 -0700 Message-ID: Subject: Re: git: 33a2406eed00 - main - reboot: Use posix_spawn instead of system To: Konstantin Belousov Cc: Warner Losh , src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: multipart/alternative; boundary="0000000000002ab542061184fcaa" X-Spamd-Bar: ---- X-Rspamd-Queue-Id: 4Tc2Tg0NdXz4H6L X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US] --0000000000002ab542061184fcaa Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, Feb 16, 2024 at 6:34=E2=80=AFAM Konstantin Belousov wrote: > On Fri, Feb 16, 2024 at 04:00:19AM +0000, Warner Losh wrote: > > The branch main has been updated by imp: > > > > URL: > https://cgit.FreeBSD.org/src/commit/?id=3D33a2406eed009dbffd7dfa44285c23f= 0db5a3750 > > > > commit 33a2406eed009dbffd7dfa44285c23f0db5a3750 > > Author: Warner Losh > > AuthorDate: 2024-02-16 03:52:31 +0000 > > Commit: Warner Losh > > CommitDate: 2024-02-16 03:59:22 +0000 > > > > reboot: Use posix_spawn instead of system > > > > Use posix_spawn to avoid having to allocate memory needed for the > system > > command line. > > > > Sponsored by: Netflix > > Reviewed by: jrtc27 > > Differential Revision: https://reviews.freebsd.org/D43860 > > --- > > sbin/reboot/reboot.c | 54 > ++++++++++++++++++++++++++++++++++++---------------- > > 1 file changed, 38 insertions(+), 16 deletions(-) > > > > diff --git a/sbin/reboot/reboot.c b/sbin/reboot/reboot.c > > index e9d6487da6a5..d3f1fc9bbb86 100644 > > --- a/sbin/reboot/reboot.c > > +++ b/sbin/reboot/reboot.c > > @@ -29,19 +29,21 @@ > > * SUCH DAMAGE. > > */ > > > > -#include > > #include > > #include > > #include > > #include > > #include > > #include > > +#include > > +#include > sys/types.h should go first, the previous location was right > Yea, it's not needed at all, so I'm removing it. > > > > #include > > #include > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -69,23 +71,43 @@ static bool donextboot; > > static void > > zfsbootcfg(const char *pool, bool force) > > { > > - char *k; > > - int rv; > > - > > - asprintf(&k, > > - "zfsbootcfg -z %s -n freebsd:nvstore -k nextboot_enable -v > YES", > > - pool); > > - if (k =3D=3D NULL) > > - E("No memory for zfsbootcfg"); > > - > > - rv =3D system(k); > > - if (rv =3D=3D 0) > > - return; > > + const char * const av[] =3D { > Why not 'static'? > > > + "zfsbootcfg", > > + "-z", > > + pool, > > + "-n", > > + "freebsd:nvstore", > > + "-k", > > + "nextboot_enable" > > + "-v", > > + "YES", > > + NULL > > + }; > Because 'pool' is not a compile time constant? Warner > > + int rv, status; > > + pid_t p; > > + extern char **environ; > > + > > + rv =3D posix_spawnp(&p, av[0], NULL, NULL, __DECONST(char **, av)= , > > + environ); > > if (rv =3D=3D -1) > > E("system zfsbootcfg"); > > - if (rv =3D=3D 127) > > - E("zfsbootcfg not found in path"); > > - E("zfsbootcfg returned %d", rv); > > + if (waitpid(p, &status, WEXITED) < 0) { > > + if (errno =3D=3D EINTR) > > + return; > > + E("waitpid zfsbootcfg"); > > + } > > + if (WIFEXITED(status)) { > > + int e =3D WEXITSTATUS(status); > > + > > + if (e =3D=3D 0) > > + return; > > + if (e =3D=3D 127) > > + E("zfsbootcfg not found in path"); > > + E("zfsbootcfg returned %d", e); > > + } > > + if (WIFSIGNALED(status)) > > + E("zfsbootcfg died with signal %d", WTERMSIG(status)); > > + E("zfsbootcfg unexpected status %d", status); > > } > > > > static void > --0000000000002ab542061184fcaa Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Fri, Feb 16, 2024 at 6:34=E2=80=AF= AM Konstantin Belousov <kostikbel= @gmail.com> wrote:
On Fri, Feb 16, 2024 at 04:00:19AM +0000, Warner Losh wrote:
> The branch main has been updated by imp:
>
> URL: https://= cgit.FreeBSD.org/src/commit/?id=3D33a2406eed009dbffd7dfa44285c23f0db5a3750<= /a>
>
> commit 33a2406eed009dbffd7dfa44285c23f0db5a3750
> Author:=C2=A0 =C2=A0 =C2=A0Warner Losh <imp@FreeBSD.org>
> AuthorDate: 2024-02-16 03:52:31 +0000
> Commit:=C2=A0 =C2=A0 =C2=A0Warner Losh <imp@FreeBSD.org>
> CommitDate: 2024-02-16 03:59:22 +0000
>
>=C2=A0 =C2=A0 =C2=A0reboot: Use posix_spawn instead of system
>=C2=A0 =C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0Use posix_spawn to avoid having to allocate memory = needed for the system
>=C2=A0 =C2=A0 =C2=A0command line.
>=C2=A0 =C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0Sponsored by:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0Netflix
>=C2=A0 =C2=A0 =C2=A0Reviewed by:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 jrtc27
>=C2=A0 =C2=A0 =C2=A0Differential Revision:=C2=A0 https://revie= ws.freebsd.org/D43860
> ---
>=C2=A0 sbin/reboot/reboot.c | 54 ++++++++++++++++++++++++++++++++++++--= --------------
>=C2=A0 1 file changed, 38 insertions(+), 16 deletions(-)
>
> diff --git a/sbin/reboot/reboot.c b/sbin/reboot/reboot.c
> index e9d6487da6a5..d3f1fc9bbb86 100644
> --- a/sbin/reboot/reboot.c
> +++ b/sbin/reboot/reboot.c
> @@ -29,19 +29,21 @@
>=C2=A0 =C2=A0* SUCH DAMAGE.
>=C2=A0 =C2=A0*/
>=C2=A0
> -#include <sys/types.h>
>=C2=A0 #include <sys/boottrace.h>
>=C2=A0 #include <sys/mount.h>
>=C2=A0 #include <sys/reboot.h>
>=C2=A0 #include <sys/stat.h>
>=C2=A0 #include <sys/sysctl.h>
>=C2=A0 #include <sys/time.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
sys/types.h should go first, the previous location was right

Yea, it's not needed at all, so I'm removing = it.
=C2=A0
>=C2=A0
>=C2=A0 #include <err.h>
>=C2=A0 #include <errno.h>
>=C2=A0 #include <fcntl.h>
>=C2=A0 #include <pwd.h>
>=C2=A0 #include <signal.h>
> +#include <spawn.h>
>=C2=A0 #include <stdbool.h>
>=C2=A0 #include <stdio.h>
>=C2=A0 #include <stdlib.h>
> @@ -69,23 +71,43 @@ static bool donextboot;
>=C2=A0 static void
>=C2=A0 zfsbootcfg(const char *pool, bool force)
>=C2=A0 {
> -=C2=A0 =C2=A0 =C2=A0char *k;
> -=C2=A0 =C2=A0 =C2=A0int rv;
> -
> -=C2=A0 =C2=A0 =C2=A0asprintf(&k,
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"zfsbootcfg -z %s -n freebsd:n= vstore -k nextboot_enable -v YES",
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0pool);
> -=C2=A0 =C2=A0 =C2=A0if (k =3D=3D NULL)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0E("No memory for= zfsbootcfg");
> -
> -=C2=A0 =C2=A0 =C2=A0rv =3D system(k);
> -=C2=A0 =C2=A0 =C2=A0if (rv =3D=3D 0)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return;
> +=C2=A0 =C2=A0 =C2=A0const char * const av[] =3D {
Why not 'static'?

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"zfsbootcfg"= ;,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"-z",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0pool,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"-n",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"freebsd:nvstore= ",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"-k",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"nextboot_enable= "
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"-v",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"YES",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0NULL
> +=C2=A0 =C2=A0 =C2=A0};

Because &#= 39;pool' is not a compile time constant?

Warne= r
=C2=A0
> +=C2=A0 =C2=A0 =C2=A0int rv, status;
> +=C2=A0 =C2=A0 =C2=A0pid_t p;
> +=C2=A0 =C2=A0 =C2=A0extern char **environ;
> +
> +=C2=A0 =C2=A0 =C2=A0rv =3D posix_spawnp(&p, av[0], NULL, NULL, __= DECONST(char **, av),
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0environ);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (rv =3D=3D -1)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0E("system z= fsbootcfg");
> -=C2=A0 =C2=A0 =C2=A0if (rv =3D=3D 127)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0E("zfsbootcfg no= t found in path");
> -=C2=A0 =C2=A0 =C2=A0E("zfsbootcfg returned %d", rv);
> +=C2=A0 =C2=A0 =C2=A0if (waitpid(p, &status, WEXITED) < 0) { > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (errno =3D=3D EINT= R)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0return;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0E("waitpid zfsbo= otcfg");
> +=C2=A0 =C2=A0 =C2=A0}
> +=C2=A0 =C2=A0 =C2=A0if (WIFEXITED(status)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0int e =3D WEXITSTATUS= (status);
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (e =3D=3D 0)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0return;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (e =3D=3D 127)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0E("zfsbootcfg not found in path");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0E("zfsbootcfg re= turned %d", e);
> +=C2=A0 =C2=A0 =C2=A0}
> +=C2=A0 =C2=A0 =C2=A0if (WIFSIGNALED(status))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0E("zfsbootcfg di= ed with signal %d", WTERMSIG(status));
> +=C2=A0 =C2=A0 =C2=A0E("zfsbootcfg unexpected status %d", st= atus);
>=C2=A0 }
>=C2=A0
>=C2=A0 static void
--0000000000002ab542061184fcaa--