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

Submitted by:  None
Submitted on:  Thu 03 Nov 2016 05:04:49 PM UTC  
 
Category: Octave FunctionSeverity: 1 - Wish
Priority: 5 - NormalItem Group: Segfault, Bus Error, etc.
Status: ConfirmedAssigned to: None
Originator Name: Originator Email: -unavailable-
Open/Closed: OpenRelease: dev
Operating System: GNU/Linux

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission 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>
Project 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>
Project 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>
Project Administrator
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

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>
Project Administrator
Thu 03 Nov 2016 10:22:55 PM UTC, comment #1:

I tested and got

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>
Project Administrator
Thu 03 Nov 2016 05:04:49 PM UTC, original submission:
Anonymous

 

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

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 07 Nov 2016 04:02:19 PM UTCrik5Severity3 - 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
    Fri 04 Nov 2016 06:37:17 PM UTCmtmillerStatusNone=>Patch Submitted

    Back to the top


    Powered by Savane 3.1-cleanup1