failed exec() exit codes

failed exec() exit codes

[Karel Zak]

Now we use on many places where commands executing another command:

    execvp(argv[optind], argv + optind);
    err(EXIT_FAILURE, _("failed to execute %s"), argv[optind]);

the problem is that EXIT_FAILURE is too generic and it does not
provide any way how to detect failed exec()s.

I have added a new macro errexec() to replace err(EXIT_FAILURE, ...).

The new macro is compatible with coreutils way:

    #define EX_EXEC_FAILED          126     /* Program located, but not usable. */
    #define EX_EXEC_ENOENT          127     /* Could not find program to exec.  */

    #define errexec(name)   err(errno == ENOENT ? EX_EXEC_ENOENT : EX_EXEC_FAILED, \ 
                        _("failed to execute %s"), name)

Example:

        $ taskset -c 1 ./foo; echo $?
        taskset: failed to execute ./foo: No such file or directory
        127

        $ touch foo

        $ taskset -c 1 ./foo; echo $?
        taskset: failed to execute ./foo: Permission denied
        126


We had discussion about it 2 years ago, but it has never been
implemented and merged. Fixed now ;-)

Objections? 

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: failed exec() exit codes

[Ruediger Meier]

On Thursday 01 February 2018, Karel Zak wrote:
> Now we use on many places where commands executing another command:
>
>     execvp(argv[optind], argv + optind);
>     err(EXIT_FAILURE, _("failed to execute %s"), argv[optind]);
>
> the problem is that EXIT_FAILURE is too generic and it does not
> provide any way how to detect failed exec()s.

I think you did a bit too much. I assume that these exec codes are only 
useful for the programs which get a "command" from the user's command 
line. So that the user can expect the original exit code of his 
wanted "command" if and only if it's not 126 or 127.

So for taskset(1) these exit codes are good but maybe not needed for 
swapon(1) or eject(1).


> I have added a new macro errexec() to replace err(EXIT_FAILURE, ...).
>
> The new macro is compatible with coreutils way:
>
>     #define EX_EXEC_FAILED          126     /* Program located, but
> not usable. */ #define EX_EXEC_ENOENT          127     /* Could not
> find program to exec.  */


coreutils has even 2 more in "src/system.h" and each of these kind of
exec programs are *only* using these ones.

/* Exit statuses for programs like 'env' that exec other programs.  */
enum
{
  EXIT_TIMEDOUT = 124, /* Time expired before child completed.  */
  EXIT_CANCELED = 125, /* Internal error prior to exec attempt.  */
  EXIT_CANNOT_INVOKE = 126, /* Program located, but not usable.  */
  EXIT_ENOENT = 127 /* Could not find program to exec.  */
};

like
$ chroot   --helpdassdf; echo $?
chroot: unrecognized option '--helpdassdf'
Try 'chroot --help' for more information.
125

Other projects like bash only support 126 and 127, like we do now

$ exec bla ; echo $?
bash: exec: bla: not found
127

$ exec --bla ; echo $?
bash: exec: --: invalid option
exec: usage: exec [-cl] [-a name] [command [arguments ...]] 
[redirection ...]
2

cu,
Rudi

Re: failed exec() exit codes

[Karel Zak]

On Thu, Feb 01, 2018 at 05:01:19PM +0100, Ruediger Meier wrote:
> On Thursday 01 February 2018, Karel Zak wrote:
> > Now we use on many places where commands executing another command:
> >
> >     execvp(argv[optind], argv + optind);
> >     err(EXIT_FAILURE, _("failed to execute %s"), argv[optind]);
> >
> > the problem is that EXIT_FAILURE is too generic and it does not
> > provide any way how to detect failed exec()s.
> 
> I think you did a bit too much. I assume that these exec codes are only 
> useful for the programs which get a "command" from the user's command 
> line. So that the user can expect the original exit code of his 
> wanted "command" if and only if it's not 126 or 127.
> 
> So for taskset(1) these exit codes are good but maybe not needed for 
> swapon(1) or eject(1).

I know, but it seems no problem and it's good way to consolidate the
error message and use the came code pattern.

> > I have added a new macro errexec() to replace err(EXIT_FAILURE, ...).
> >
> > The new macro is compatible with coreutils way:
> >
> >     #define EX_EXEC_FAILED          126     /* Program located, but
> > not usable. */ #define EX_EXEC_ENOENT          127     /* Could not
> > find program to exec.  */
> 
> 
> coreutils has even 2 more in "src/system.h" and each of these kind of
> exec programs are *only* using these ones.

I have copied from this file ;-)

> 
> /* Exit statuses for programs like 'env' that exec other programs.  */
> enum
> {
>   EXIT_TIMEDOUT = 124, /* Time expired before child completed.  */
>   EXIT_CANCELED = 125, /* Internal error prior to exec attempt.  */
>   EXIT_CANNOT_INVOKE = 126, /* Program located, but not usable.  */
>   EXIT_ENOENT = 127 /* Could not find program to exec.  */
> };
> 
> like
> $ chroot   --helpdassdf; echo $?
> chroot: unrecognized option '--helpdassdf'
> Try 'chroot --help' for more information.
> 125
> 
> Other projects like bash only support 126 and 127, like we do now

yes, I guess it's good enough.

    Karel
-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: failed exec() exit codes

[Rüdiger Meier]

On 02/01/18 21:22, Karel Zak wrote:
> On Thu, Feb 01, 2018 at 05:01:19PM +0100, Ruediger Meier wrote:
>> On Thursday 01 February 2018, Karel Zak wrote:
>>> Now we use on many places where commands executing another command:
>>>
>>>      execvp(argv[optind], argv + optind);
>>>      err(EXIT_FAILURE, _("failed to execute %s"), argv[optind]);
>>>
>>> the problem is that EXIT_FAILURE is too generic and it does not
>>> provide any way how to detect failed exec()s.
>>
>> I think you did a bit too much. I assume that these exec codes are only
>> useful for the programs which get a "command" from the user's command
>> line. So that the user can expect the original exit code of his
>> wanted "command" if and only if it's not 126 or 127.
>>
>> So for taskset(1) these exit codes are good but maybe not needed for
>> swapon(1) or eject(1).
> 
> I know, but it seems no problem and it's good way to consolidate the
> error message and use the came code pattern.

But isn't it just an implementation detail that we exec umount(1)? If we
would replace that later by a library call, should we change the exit code
again?

>>> I have added a new macro errexec() to replace err(EXIT_FAILURE, ...).
>>>
>>> The new macro is compatible with coreutils way:
>>>
>>>      #define EX_EXEC_FAILED          126     /* Program located, but
>>> not usable. */ #define EX_EXEC_ENOENT          127     /* Could not
>>> find program to exec.  */
>>
>>
>> coreutils has even 2 more in "src/system.h" and each of these kind of
>> exec programs are *only* using these ones.
> 
> I have copied from this file ;-)
> 
>>
>> /* Exit statuses for programs like 'env' that exec other programs.  */
>> enum
>> {
>>    EXIT_TIMEDOUT = 124, /* Time expired before child completed.  */
>>    EXIT_CANCELED = 125, /* Internal error prior to exec attempt.  */
>>    EXIT_CANNOT_INVOKE = 126, /* Program located, but not usable.  */
>>    EXIT_ENOENT = 127 /* Could not find program to exec.  */
>> };
>>
>> like
>> $ chroot   --helpdassdf; echo $?
>> chroot: unrecognized option '--helpdassdf'
>> Try 'chroot --help' for more information.
>> 125
>>
>> Other projects like bash only support 126 and 127, like we do now
> 
> yes, I guess it's good enough.

Probably you are right, but rock solid error handling would only be possible
if we do it like coreutils' timeout for example. See "bad case 2" below. If
"timeout inval" would return 1 then one might think that the pattern was not
in the input. ;)

# normal case
$ echo "xyz" | timeout 10 grep -q "bla" ; echo $?
1

# bad case 1
$ echo "bla" | timeout 10 xxxgrep -q "bla" ; echo $?
timeout: failed to run command ‘xxxgrep’: No such file or directory
127

# bad case 2
$ echo "bla" | timeout inval  xxxgrep -q "bla" ; echo $?
timeout: invalid time interval ‘inval’
Try 'timeout --help' for more information.
125

cu,
Rudi

Re: failed exec() exit codes

[Karel Zak]

On Thu, Feb 01, 2018 at 10:31:55PM +0100, Rüdiger Meier wrote:
> 
> 
> On 02/01/18 21:22, Karel Zak wrote:
> > On Thu, Feb 01, 2018 at 05:01:19PM +0100, Ruediger Meier wrote:
> > > On Thursday 01 February 2018, Karel Zak wrote:
> > > > Now we use on many places where commands executing another command:
> > > > 
> > > >      execvp(argv[optind], argv + optind);
> > > >      err(EXIT_FAILURE, _("failed to execute %s"), argv[optind]);
> > > > 
> > > > the problem is that EXIT_FAILURE is too generic and it does not
> > > > provide any way how to detect failed exec()s.
> > > 
> > > I think you did a bit too much. I assume that these exec codes are only
> > > useful for the programs which get a "command" from the user's command
> > > line. So that the user can expect the original exit code of his
> > > wanted "command" if and only if it's not 126 or 127.
> > > 
> > > So for taskset(1) these exit codes are good but maybe not needed for
> > > swapon(1) or eject(1).
> > 
> > I know, but it seems no problem and it's good way to consolidate the
> > error message and use the came code pattern.
> 
> But isn't it just an implementation detail that we exec umount(1)? If we
> would replace that later by a library call, should we change the exit code
> again?

No, see the code. The exit code is used in our child and handled by
our waitpid(). It's not used as exit code for whole eject or swapon.
There is no visible change for users.

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com