bugGNU Octave - Bugs: bug #49525, popen2 should return an error code...

 
 

bug #49525: popen2 should return an error code (-1) if command does not exist

Submitter:  None
Submitted:  Thu 03 Nov 2016 05:04:49 PM UTC
   
 
Category:  Octave Function Severity:  1 - Wish
Priority:  5 - Normal Item Group:  Feature Request
Status:  Confirmed Assigned to:  None
Originator Name:  Originator Email:  -email is unavailable-
Open/Closed:  * Open Release:  * dev
Operating System:  * GNU/Linux Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 07 Nov 2016 04:02:19 PM UTC, comment #6: 

@Mike: I pushed your patch to the stable branch here http://hg.savannah.gnu.org/hgweb/octave/rev/838d52a3283e.

I'm re-titling the report to reflect the remaining issue, and also making this a wish list item.

Rik <rik5>
Group administrator
Fri 04 Nov 2016 06:47:09 PM UTC, comment #5: 

@Mike: This seems right.  Why don't you commit it.  I think it can go on stable too since it fixes a segfault.


Rik <rik5>
Group administrator
Fri 04 Nov 2016 06:37:17 PM UTC, comment #4: 

Thanks, marking as working patch submitted.

Returning a failure indicating that the child process couldn't start seems like it's almost an unsolvable problem to me. Maybe we could open a third pipe between the parent and the child, set the close-on-exec flag, and use it to send a failure message back to Octave if exec()ing the child process fails?

Since the user who calls popen2 is expected to communicate with the child and use waitpid to check status / reap the child anyway, this seems like a separate feature request, but I think this bug can be closed with the simple change from exit to _exit (and I chose 127 because that's what bash returns for command not found).

Mike Miller <mtmiller>
Group Member
Fri 04 Nov 2016 05:58:52 PM UTC, comment #3: 

@Mike: No more segfault. Perhaps it would be nice if the returned pid is -1 in this case.

Anonymous
Fri 04 Nov 2016 04:46:28 PM UTC, comment #2: 

Making this simple change eliminates the "Inconsistency detected" message for me


diff --git a/liboctave/wrappers/octave-popen2.c b/liboctave/wrappers/octave-popen2.c
--- a/liboctave/wrappers/octave-popen2.c
+++ b/liboctave/wrappers/octave-popen2.c
@@ -206,7 +206,7 @@ octave_popen2 (const char *cmd, char *co
       else
         perror ("popen2 (child)");

-      exit (0);
+      _exit (127);
     }
   else if (pid > 0)
     {


Not the exit code, but the use of _exit instead of exit.

The message is coming from the child process as it tries to exit and call dlclose on something (not sure what, but some shared objects that Octave has opened). If exec() had succeeded, it would not normally run all the cleanup that happens when exit() is called, so I think calling _exit() would be safer anyway.

I also think it would be nice if popen2 could return a failure if the child fails to exec, but I'm not sure how to do that in a safe way.

Anonymous, if you change exit to _exit does that fix the segfault on your system?

Mike Miller <mtmiller>
Group Member
Thu 03 Nov 2016 10:22:55 PM UTC, comment #1: 

I tested and got


[in,out,pid] = popen2 ('abc')
in =  6
out =  7
pid =  10709
popen2 (child): No such file or directory
octave:6> Inconsistency detected by ld.so: dl-close.c: 764: _dl_close: Assertion `map->l_init_called' failed!


So there is an inconsistency, but I don't get a segfault.  From the documentation, I would expect pid to be -1 if the command failed, but it is the valid id of the forked child (although the child is defunct immediately because the shell throws an error for 'abc').

This situation isn't handled well even in C according to the man page for popen .  I don't know what to do here, but it would be nice to have something better than a segfault.

Rik <rik5>
Group administrator
Thu 03 Nov 2016 05:04:49 PM UTC, original submission:  


octave:1> popen2 ("foo")
ans =  15
octave:2> popen2 (child): No such file or directory
panic: Segmentation fault -- stopping myself...
attempting to save variables to 'octave-workspace'...
save to 'octave-workspace' complete


Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-12-22 mtmiller Carbon-CopyRemoved 80942 -
    2018-12-22 rik5 Item GroupSegfault, Bus Error, etc. Feature Request
    2016-11-07 rik5 Severity3 - Normal 1 - Wish
        StatusPatch Submitted Confirmed
        Release4.2.0-rc2 dev
        Summarypopen2 segfaults if command doesn't exist popen2 should return an error code (-1) if command does not exist
    2016-11-04 mtmiller StatusNone Patch Submitted

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code