From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id 1FFB05A026A for ; Wed, 8 Feb 2023 19:14:30 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1675880069; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=P1WTuyescqhh3XPvWgG55ybjL9PC7YpsicI7KNP1NF0=; b=AyJ9ErqN91G5hE8oipZh3wR30xdKuwWtQnX0Kbpsrm6M9qT0IwxyI+RnmsKeAG2HmMdRgi Zk6tDIeuWI3YGobPaNeQlB/7aXQNXvSwp947ONRlfuRqhXOmUyYn+2SJxi0Mdz9i+M+hJ1 uE9jPAYXncFvS0jODWSk5098xnR3Y5g= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-402-iiAmTjl1PiuvaA7tlIp5qA-1; Wed, 08 Feb 2023 13:14:28 -0500 X-MC-Unique: iiAmTjl1PiuvaA7tlIp5qA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id D321885A5B1 for ; Wed, 8 Feb 2023 18:14:27 +0000 (UTC) Received: from maya.cloud.tilaa.com (ovpn-208-4.brq.redhat.com [10.40.208.4]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9A12C1121315; Wed, 8 Feb 2023 18:14:27 +0000 (UTC) Date: Wed, 8 Feb 2023 19:14:25 +0100 From: Stefano Brivio To: Paul Holzinger Subject: Re: [PATCH 2/2] pasta: propagate exit code from child command Message-ID: <20230208191425.743197cd@elisabeth> In-Reply-To: <20230208155441.64306-2-pholzing@redhat.com> References: <20230208155441.64306-1-pholzing@redhat.com> <20230208155441.64306-2-pholzing@redhat.com> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: P5H27LRITAZ7SRI6U5WMXC4OLMJXFU4J X-Message-ID-Hash: P5H27LRITAZ7SRI6U5WMXC4OLMJXFU4J X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.3 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Wed, 8 Feb 2023 16:54:41 +0100 Paul Holzinger wrote: > Exits codes are very useful for scripts, when the pasta child execvp() > call fails with ENOENT that parent should also exit with > 0. In short > the parent should always exit with the code from the child to make it > useful in scripts. > > It is easy to test with: `pasta -- bash -c "exit 3"; echo $?` > > Signed-off-by: Paul Holzinger > --- > pasta.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/pasta.c b/pasta.c > index 3f6477c..4b18d7e 100644 > --- a/pasta.c > +++ b/pasta.c > @@ -64,9 +64,14 @@ void pasta_child_handler(int signal) > > if (pasta_child_pid && > !waitid(P_PID, pasta_child_pid, &infop, WEXITED | WNOHANG)) { > - if (infop.si_pid == pasta_child_pid) > - exit(EXIT_SUCCESS); > - /* Nothing to do, detached PID namespace going away */ > + if (infop.si_pid == pasta_child_pid) { > + if (infop.si_code == CLD_EXITED) > + exit(infop.si_status); > + > + /* else killed by signal, si_status == SIGNUM in this case */ It took me a while to figure out that SIGNUM is not a constant I wasn't aware of, just the signal number -- I guess you meant 'signum' from sigaction() or suchlike? Maybe you could go with: /* If killed by a signal, si_status is its number */ > + exit(infop.si_status + 128); This is a bit arbitrary -- returning n + 128 is what Bash, zsh, and a few other shells do, and probably a reasonable convention given that POSIX says we need to return a value greater than 128. Still I would mention it: /* Arbitrary: return signal number + 128 */ or: /* If killed by a signal, si_status is the number. * Follow common shell convention of returning it + 128. */ > + } > + /* Nothing to do, detached PID namespace going away */ This should be moved back into the conditional block (swapping the two lines above, or moving this comment at the beginning of the block). If si_pid is not the child, we need to do other stuff (that is, reap clones). > } > > waitid(P_ALL, 0, NULL, WEXITED | WNOHANG); -- Stefano