public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] posix: Don't automatically fail if _LIBC_REENTRANT isn't defined.
@ 2020-09-23 10:51 Efraim Flashner
  2020-09-23 11:56 ` Florian Weimer
  0 siblings, 1 reply; 2+ messages in thread
From: Efraim Flashner @ 2020-09-23 10:51 UTC (permalink / raw)
  To: libc-alpha; +Cc: Adhemerval Zanella, Efraim Flashner

If status is instantiated as -1 then do_system() will always fail in
some circumstances.
---

I was working on porting GNU Guix to powerpc-linux and I stumbled across
what I'm assuming is a bug. As part of building the bootstrap binaries
to build the entire system a static copy of guile is created, using a
static copy of glibc. When this copy of guile was used inside a Guix
build container the 'system' command from guile would unconditionally
error. 'system' uses the default 'sh'
(https://www.gnu.org/software/guile/manual/html_node/Processes.html#index-system)
which, for the static glibc, we've patched to be 'sh' and not '/bin/sh'
so guile can find it in the environment, which intentionally lacks /bin/sh.
This has worked for all the other architectures we support.

When I was looking through code to see what might've changed I realized
that most instances of status were changed to ret, and status was now
initialized to -1. By either reverting [BZ #25715]
f09542c584b121da0322fde4b55306d512b85d93 or by initializing status to 0
I was able to continue with the porting effort.

---
 sysdeps/posix/system.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sysdeps/posix/system.c b/sysdeps/posix/system.c
index a03f478fc7..19d07d2581 100644
--- a/sysdeps/posix/system.c
+++ b/sysdeps/posix/system.c
@@ -101,7 +101,7 @@ cancel_handler (void *arg)
 static int
 do_system (const char *line)
 {
-  int status = -1;
+  int status = 0;
   int ret;
   pid_t pid;
   struct sigaction sa;
-- 
2.28.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] posix: Don't automatically fail if _LIBC_REENTRANT isn't defined.
  2020-09-23 10:51 [PATCH] posix: Don't automatically fail if _LIBC_REENTRANT isn't defined Efraim Flashner
@ 2020-09-23 11:56 ` Florian Weimer
  0 siblings, 0 replies; 2+ messages in thread
From: Florian Weimer @ 2020-09-23 11:56 UTC (permalink / raw)
  To: Efraim Flashner; +Cc: libc-alpha

* Efraim Flashner:

> diff --git a/sysdeps/posix/system.c b/sysdeps/posix/system.c
> index a03f478fc7..19d07d2581 100644
> --- a/sysdeps/posix/system.c
> +++ b/sysdeps/posix/system.c
> @@ -101,7 +101,7 @@ cancel_handler (void *arg)
>  static int
>  do_system (const char *line)
>  {
> -  int status = -1;
> +  int status = 0;
>    int ret;
>    pid_t pid;
>    struct sigaction sa;

I don't think this is correct.  You are just hiding an error return from
posix_spawn, I think.

A test case (against unpatched glibc) would help to clarify what you are
looking for.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-09-23 11:56 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-23 10:51 [PATCH] posix: Don't automatically fail if _LIBC_REENTRANT isn't defined Efraim Flashner
2020-09-23 11:56 ` Florian Weimer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).