From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oo1-xc36.google.com (mail-oo1-xc36.google.com [IPv6:2607:f8b0:4864:20::c36]) by sourceware.org (Postfix) with ESMTPS id 9CA6A3858C5F for ; Thu, 9 Feb 2023 19:43:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9CA6A3858C5F Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-oo1-xc36.google.com with SMTP id k15-20020a4adfaf000000b00517450f9bd7so319579ook.8 for ; Thu, 09 Feb 2023 11:43:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=content-transfer-encoding:in-reply-to:organization:references:to :from:content-language:subject:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=NAV5j1w4iRxk7Fj0bvMqkV16wRAy5GAogjhu1S/yPaE=; b=VL1ICp3vFvg+mPnkm6zjj7PlKpCGgHTNPdd7oD9x+RmYniwYxQrUIMJhM4UwERlusE +s4sihSMKRX9rK0QNbGD2wIcYsMuOamSAOygWmSDZEIPfibILBSzm59dqcnrsuD8f8MH pGyxlGTdgg8fm3LWF05zfRAFrhlPUf/vWEv56fDqml39aAl/sezdzbfILg08XpUK8nYc OffaSG6ahbFEsbjpeNlsmUYhQi+DZSEqJ/Qz4wn/KcLJm6Pn5r2JHlAP/rn/NCCqzBqS Bax0QpOY2k55vU3mY7OobbC8i56dIX8NYlpeBsPDtsinPx/dyTeWdyH+AzpVaL1m9YRB KDTQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:organization:references:to :from:content-language:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=NAV5j1w4iRxk7Fj0bvMqkV16wRAy5GAogjhu1S/yPaE=; b=E4k4meQ57ylnQUpdZ7gUUgwHuAsBkos54NL0Yz+Y+cKLkwooQfDitzn4r/z7u07/Ni WtTc8Pd5WqMf/OwfMoeM+veqNj6roRnVETDws+B6Fb5U4hv/6j4qQ5d5iNPjgXMH6Sdp ScLgqZPYFdRjDSjhMHQRB/k9v16pLaYsHx2+Ms6RPDZrNdytHcAyMlRdG7qc9KWvU/9l uj12Apurqg5zGkoDBMt0mcjs0D47rnSwfeNict6tyW7iax6dt8jtqPNTvrA9ao3CY5rm 8cfO4mr8PBCmzsPI3Hmbm5Y+AkLfs4b48uukHibXX7r/EabNvLgkg1oUpkexOKWfHR04 njIw== X-Gm-Message-State: AO0yUKU68Ptsagc76c7FdrQl3BsvOOFOsI2bSdM9vYWGfX6AMy+QC++q cxNrNUvnXpRo8P8hBZ7J1IgMsgnj1oy0EHDUCtg= X-Google-Smtp-Source: AK7set/gBfndTWt3SQ5Yfm6TAeMwlJVDbOQJT4pyH+/sYV/S7Yu0vKdvTOUQCSQBPn3z24Wuv0lUCQ== X-Received: by 2002:a4a:5105:0:b0:4e7:128c:f195 with SMTP id s5-20020a4a5105000000b004e7128cf195mr6336554ooa.8.1675971786246; Thu, 09 Feb 2023 11:43:06 -0800 (PST) Received: from ?IPV6:2804:1b3:a7c2:8ced:4490:9a44:1abc:1757? ([2804:1b3:a7c2:8ced:4490:9a44:1abc:1757]) by smtp.gmail.com with ESMTPSA id h15-20020a4abb8f000000b004f21af81e0csm1097220oop.46.2023.02.09.11.43.03 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 09 Feb 2023 11:43:05 -0800 (PST) Message-ID: <1cfa9229-9edd-82e3-42b2-a60c9798c69a@linaro.org> Date: Thu, 9 Feb 2023 16:43:02 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [PATCH v2] stdio: Do not ignore posix_spawn error on popen (BZ #29016) Content-Language: en-US From: Adhemerval Zanella Netto To: libc-alpha@sourceware.org, Andreas Schwab , arnaud.lb@gmail.com References: <20221207182438.172691-1-adhemerval.zanella@linaro.org> Organization: Linaro In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Ping (x2). On 10/01/23 12:21, Adhemerval Zanella Netto wrote: > Ping. > > On 07/12/22 15:24, Adhemerval Zanella wrote: >> To correctly return error in case of default shell is not present. >> >> Checked on x86_64-linux-gnu. >> --- >> libio/iopopen.c | 38 ++++++++++++++++++++++---------------- >> stdio-common/Makefile | 3 +++ >> stdio-common/tst-popen3.c | 38 ++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 63 insertions(+), 16 deletions(-) >> create mode 100644 stdio-common/tst-popen3.c >> >> diff --git a/libio/iopopen.c b/libio/iopopen.c >> index 06778cf110..66f5936114 100644 >> --- a/libio/iopopen.c >> +++ b/libio/iopopen.c >> @@ -66,11 +66,12 @@ unlock (void *not_used) >> be close (by transversing the proc_file_chain list) and the insertion of a >> new one after a successful posix_spawn this function should be called >> with proc_file_chain_lock acquired. */ >> -static bool >> +static int >> spawn_process (posix_spawn_file_actions_t *fa, FILE *fp, const char *command, >> int do_cloexec, int pipe_fds[2], int parent_end, int child_end, >> int child_pipe_fd) >> { >> + int err; >> >> for (struct _IO_proc_file *p = proc_file_chain; p; p = p->next) >> { >> @@ -79,15 +80,19 @@ spawn_process (posix_spawn_file_actions_t *fa, FILE *fp, const char *command, >> /* If any stream from previous popen() calls has fileno >> child_pipe_fd, it has been already closed by the adddup2 action >> above. */ >> - if (fd != child_pipe_fd >> - && __posix_spawn_file_actions_addclose (fa, fd) != 0) >> - return false; >> + if (fd != child_pipe_fd) >> + { >> + err = __posix_spawn_file_actions_addclose (fa, fd); >> + if (err != 0) >> + return err; >> + } >> } >> >> - if (__posix_spawn (&((_IO_proc_file *) fp)->pid, _PATH_BSHELL, fa, 0, >> - (char *const[]){ (char*) "sh", (char*) "-c", >> - (char *) command, NULL }, __environ) != 0) >> - return false; >> + err = __posix_spawn (&((_IO_proc_file *) fp)->pid, _PATH_BSHELL, fa, 0, >> + (char *const[]){ (char*) "sh", (char*) "-c", >> + (char *) command, NULL }, __environ); >> + if (err != 0) >> + return err; >> >> __close_nocancel (pipe_fds[child_end]); >> >> @@ -101,7 +106,7 @@ spawn_process (posix_spawn_file_actions_t *fa, FILE *fp, const char *command, >> ((_IO_proc_file *) fp)->next = proc_file_chain; >> proc_file_chain = (_IO_proc_file *) fp; >> >> - return true; >> + return 0; >> } >> >> FILE * >> @@ -112,7 +117,7 @@ _IO_new_proc_open (FILE *fp, const char *command, const char *mode) >> int parent_end, child_end; >> int pipe_fds[2]; >> int child_pipe_fd; >> - bool spawn_ok; >> + int err; >> >> int do_read = 0; >> int do_write = 0; >> @@ -185,16 +190,17 @@ _IO_new_proc_open (FILE *fp, const char *command, const char *mode) >> pipe_fds[child_end] = tmp; >> } >> >> - if (__posix_spawn_file_actions_adddup2 (&fa, pipe_fds[child_end], >> - child_pipe_fd) != 0) >> + err = __posix_spawn_file_actions_adddup2 (&fa, pipe_fds[child_end], >> + child_pipe_fd); >> + if (err != 0) >> goto spawn_failure; >> >> #ifdef _IO_MTSAFE_IO >> _IO_cleanup_region_start_noarg (unlock); >> _IO_lock_lock (proc_file_chain_lock); >> #endif >> - spawn_ok = spawn_process (&fa, fp, command, do_cloexec, pipe_fds, >> - parent_end, child_end, child_pipe_fd); >> + err = spawn_process (&fa, fp, command, do_cloexec, pipe_fds, parent_end, >> + child_end, child_pipe_fd); >> #ifdef _IO_MTSAFE_IO >> _IO_lock_unlock (proc_file_chain_lock); >> _IO_cleanup_region_end (0); >> @@ -202,12 +208,12 @@ _IO_new_proc_open (FILE *fp, const char *command, const char *mode) >> >> __posix_spawn_file_actions_destroy (&fa); >> >> - if (!spawn_ok) >> + if (err != 0) >> { >> + __set_errno (err); >> spawn_failure: >> __close_nocancel (pipe_fds[child_end]); >> __close_nocancel (pipe_fds[parent_end]); >> - __set_errno (ENOMEM); >> return NULL; >> } >> >> diff --git a/stdio-common/Makefile b/stdio-common/Makefile >> index fe57dbdf56..9a9a37ae08 100644 >> --- a/stdio-common/Makefile >> +++ b/stdio-common/Makefile >> @@ -215,6 +215,9 @@ tests := \ >> xbug \ >> # tests >> >> +tests-container += \ >> + tst-popen3 >> + >> generated += \ >> errlist-data-aux-shared.S \ >> errlist-data-aux.S \ >> diff --git a/stdio-common/tst-popen3.c b/stdio-common/tst-popen3.c >> new file mode 100644 >> index 0000000000..47f4c8cd91 >> --- /dev/null >> +++ b/stdio-common/tst-popen3.c >> @@ -0,0 +1,38 @@ >> +/* Check if popen returns a correct error code if the default shell does not >> + exist (BZ#29016). >> + >> + Copyright (C) 2022 Free Software Foundation, Inc. >> + This file is part of the GNU C Library. >> + >> + The GNU C Library is free software; you can redistribute it and/or >> + modify it under the terms of the GNU Lesser General Public >> + License as published by the Free Software Foundation; either >> + version 2.1 of the License, or (at your option) any later version. >> + >> + The GNU C Library is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + Lesser General Public License for more details. >> + >> + You should have received a copy of the GNU Lesser General Public >> + License along with the GNU C Library; if not, see >> + . */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +int >> +do_test (void) >> +{ >> + xunlink (_PATH_BSHELL); >> + >> + FILE *f = popen ("/non-existent", "r"); >> + TEST_VERIFY (f == NULL); >> + TEST_COMPARE (errno, ENOENT); >> + return 0; >> +} >> + >> +#include