diff mbox series

libiberty: Fix error return value in pex_unix_exec_child [PR113957].

Message ID 20240216145643.50448-1-iain@sandoe.co.uk
State New
Headers show
Series libiberty: Fix error return value in pex_unix_exec_child [PR113957]. | expand

Commit Message

Iain Sandoe Feb. 16, 2024, 2:56 p.m. UTC
tested on x86_64-darwin, so far. OK for trunk if regression test is
successful on Linux too?
thanks
Iain

--- 8< ---

r14-5310-g879cf9ff45d940 introduced some new handling for spawning sub
processes.  The return value from the generic exec_child is examined
and needs to be < 0 to signal an error. However, the unix flavour of
this routine is returning the PID value set from the posix_spawn{p}.

This latter value is undefined per the manual pages for both Darwin
and Linux, and it seems Darwin, at least, sets the value to some
usually positive number (presumably the PID that would have been used
if the fork had succeeded).

The fix proposed here is to set the pid = -1 in the relevant error
paths.

	PR other/113957

libiberty/ChangeLog:

	* pex-unix.c (pex_unix_exec_child): Set pid = -1 in the error
	paths, since that is used to signal an erroneous outcome for
	the routine.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
---
 libiberty/pex-unix.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jeff Law Feb. 16, 2024, 3:49 p.m. UTC | #1
On 2/16/24 07:56, Iain Sandoe wrote:
> tested on x86_64-darwin, so far. OK for trunk if regression test is
> successful on Linux too?
> thanks
> Iain
> 
> --- 8< ---
> 
> r14-5310-g879cf9ff45d940 introduced some new handling for spawning sub
> processes.  The return value from the generic exec_child is examined
> and needs to be < 0 to signal an error. However, the unix flavour of
> this routine is returning the PID value set from the posix_spawn{p}.
> 
> This latter value is undefined per the manual pages for both Darwin
> and Linux, and it seems Darwin, at least, sets the value to some
> usually positive number (presumably the PID that would have been used
> if the fork had succeeded).
> 
> The fix proposed here is to set the pid = -1 in the relevant error
> paths.
> 
> 	PR other/113957
> 
> libiberty/ChangeLog:
> 
> 	* pex-unix.c (pex_unix_exec_child): Set pid = -1 in the error
> 	paths, since that is used to signal an erroneous outcome for
> 	the routine.
I wonder if that's what's been causing the native emulated PA builds to 
fail.  It's definitely related to that code.  Never saw it on any of the 
other native emulated targets through.

OK
jeff
diff mbox series

Patch

diff --git a/libiberty/pex-unix.c b/libiberty/pex-unix.c
index af98062a94c..f3a1cc95ada 100644
--- a/libiberty/pex-unix.c
+++ b/libiberty/pex-unix.c
@@ -695,6 +695,7 @@  pex_unix_exec_child (struct pex_obj *obj ATTRIBUTE_UNUSED,
 	{
 	  *err = ret;
 	  *errmsg = "posix_spawnp";
+	  pid = -1; /* The value of pid is unspecified on failure.  */
 	  goto exit;
 	}
     }
@@ -705,6 +706,7 @@  pex_unix_exec_child (struct pex_obj *obj ATTRIBUTE_UNUSED,
 	{
 	  *err = ret;
 	  *errmsg = "posix_spawn";
+	  pid = -1;
 	  goto exit;
 	}
     }