Re: [PATCH]exit.c: support larger exit code

Re: [PATCH]exit.c: support larger exit code

[Oleg Nesterov]

On 08/06, Zhang, Wei-Jovi (NSN - CN/Hangzhou) wrote:
>
> Nowadays userspace application use systemcall exit/exit_group only
> support one byte exit code.
> In some cases this exit code range is too small for some "big
> application"(like telecom software, 255 is not enough).
>
> So we can give some "big application" a chance to get larger exit code
> from child process.
> For other application don't want use larger exit code, they can use
> marco WEXITSTATUS to get lower one byte exit code.
>
> #define WEXITSTATUS(status)   __WEXITSTATUS (__WAIT_INT (status))
> --- stdlib.h
> #define       __WEXITSTATUS(status)   (((status) & 0xff00) >> 8)
> --- usrbits/waitstatus.h
>
>
> diff --git a/kernel/exit.c b/kernel/exit.c
> index ceffc67..8b13676 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -1045,7 +1045,7 @@ EXPORT_SYMBOL(complete_and_exit);
>
>  SYSCALL_DEFINE1(exit, int, error_code)
>  {
> -       do_exit((error_code&0xff)<<8);
> +       do_exit(error_code << 8);
>  }
>
>  /*
> @@ -1086,7 +1086,7 @@ do_group_exit(int exit_code)
>   */
>  SYSCALL_DEFINE1(exit_group, int, error_code)
>  {
> -       do_group_exit((error_code & 0xff) << 8);
> +       do_group_exit(error_code << 8);
>         /* NOTREACHED */
>         return 0;
>  }

Hmm. Looking at this patch, I am wondering what was the reason for the
current one-byte limitation.

I think the patch is fine. si_status, wo_stat are int too, so I do not
see any possibility for truncation before reporting to user-space.

Oleg.

Re: [PATCH]exit.c: support larger exit code

[Alexander Clouter]

Oleg Nesterov <oleg@redhat.com> wrote:
>>
>> Nowadays userspace application use systemcall exit/exit_group only
>> support one byte exit code.
>> In some cases this exit code range is too small for some "big
>> application"(like telecom software, 255 is not enough).
>>
>> So we can give some "big application" a chance to get larger exit code
>> from child process.
>> For other application don't want use larger exit code, they can use
>> marco WEXITSTATUS to get lower one byte exit code.
>>
>> #define WEXITSTATUS(status)   __WEXITSTATUS (__WAIT_INT (status))
>> --- stdlib.h
>> #define       __WEXITSTATUS(status)   (((status) & 0xff00) >> 8)
>> --- usrbits/waitstatus.h
>>
>>
>> diff --git a/kernel/exit.c b/kernel/exit.c
>> index ceffc67..8b13676 100644
>> --- a/kernel/exit.c
>> +++ b/kernel/exit.c
>> @@ -1045,7 +1045,7 @@ EXPORT_SYMBOL(complete_and_exit);
>>
>>  SYSCALL_DEFINE1(exit, int, error_code)
>>  {
>> -       do_exit((error_code&0xff)<<8);
>> +       do_exit(error_code << 8);
>>  }
>>
>>  /*
>> @@ -1086,7 +1086,7 @@ do_group_exit(int exit_code)
>>   */
>>  SYSCALL_DEFINE1(exit_group, int, error_code)
>>  {
>> -       do_group_exit((error_code & 0xff) << 8);
>> +       do_group_exit(error_code << 8);
>>         /* NOTREACHED */
>>         return 0;
>>  }
> 
> Hmm. Looking at this patch, I am wondering what was the reason for the
> current one-byte limitation.
>
The one byte limitation I think is all that is needed to give an 
impression and *hint* of what went wrong, it was not ever meant to cover 
every possible error that the child process could report.  Even "small 
programs" could generate $BIGNUM error codes it could be argued.

Looking at the list for reserved exitcodes[1] everything covers 
operational use and then if you look to /usr/include/sysexits.h for an 
idea of the 'spirit' behind exitcodes, it is pretty clear it is all 
rather 'generic' and vague to the precise reason, but it categorises 
the type of error to maybe something the parent could gracefully handle.  
It is not a freetext communications channel :)

If you have $BIGNUM outcomes of errors, you probably should not be using 
the exitcode path to communicate this information back up, although I 
would agree it looks like very natural solution.  I would be more 
inclined to pass up to the parent this information via a pipe (whether 
that is via stdout, stderr or even a filesystem located pipe).  No doubt 
if you are trying to return $BIGNUM error codes, you probably want to 
look more to an approach based on the one covered in RFC3463; there if 
you return a sysexits.h type code then the child's STDOUT is consulted 
for the extended error code reason...I think this approach is *very* 
nice.

Cheers

[1] http://tldp.org/LDP/abs/html/exitcodes.html#EXITCODESREF
[2] http://tools.ietf.org/html/rfc3463

-- 
Alexander Clouter
.sigmonster says: If in doubt, mumble.

Re: [PATCH]exit.c: support larger exit code

[Andrew Morton]

On Fri, 6 Aug 2010 14:44:56 +0200
Oleg Nesterov <oleg@redhat.com> wrote:

> On 08/06, Zhang, Wei-Jovi (NSN - CN/Hangzhou) wrote:
> >
> > Nowadays userspace application use systemcall exit/exit_group only
> > support one byte exit code.
> > In some cases this exit code range is too small for some "big
> > application"(like telecom software, 255 is not enough).
> >
> > So we can give some "big application" a chance to get larger exit code
> > from child process.
> > For other application don't want use larger exit code, they can use
> > marco WEXITSTATUS to get lower one byte exit code.
> >
> > #define WEXITSTATUS(status)   __WEXITSTATUS (__WAIT_INT (status))
> > --- stdlib.h
> > #define       __WEXITSTATUS(status)   (((status) & 0xff00) >> 8)
> > --- usrbits/waitstatus.h
> >
> >
> > diff --git a/kernel/exit.c b/kernel/exit.c
> > index ceffc67..8b13676 100644
> > --- a/kernel/exit.c
> > +++ b/kernel/exit.c
> > @@ -1045,7 +1045,7 @@ EXPORT_SYMBOL(complete_and_exit);
> >
> >  SYSCALL_DEFINE1(exit, int, error_code)
> >  {
> > -       do_exit((error_code&0xff)<<8);
> > +       do_exit(error_code << 8);
> >  }
> >
> >  /*
> > @@ -1086,7 +1086,7 @@ do_group_exit(int exit_code)
> >   */
> >  SYSCALL_DEFINE1(exit_group, int, error_code)
> >  {
> > -       do_group_exit((error_code & 0xff) << 8);
> > +       do_group_exit(error_code << 8);
> >         /* NOTREACHED */
> >         return 0;
> >  }
> 
> Hmm. Looking at this patch, I am wondering what was the reason for the
> current one-byte limitation.
> 
> I think the patch is fine. si_status, wo_stat are int too, so I do not
> see any possibility for truncation before reporting to user-space.
> 

There are back-compatibility issues.   If my crufty old child does

	exit(0xff01);

and my crufty old parent does

	if (exit_code == 1)

then I think the patch just broke my application.  The company which
wrote it no longer exists and I don't have source...

Is it worth this risk?


Also, the patch was missing a signed-off-by: and was in some complicated
html-in-mime format.

Re: [PATCH]exit.c: support larger exit code

[Tony Luck]

On Wed, Aug 25, 2010 at 2:49 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> There are back-compatibility issues.

Also standards conformance issues. Posix says this about exit():

"The value of status may be 0, EXIT_SUCCESS, EXIT_FAILURE, [CX]   or
any other value, though only the least significant 8 bits (that is,
status & 0377) shall be available to a waiting parent process."

Full reference here:
http://www.opengroup.org/onlinepubs/000095399/functions/exit.html

-Tony

Re: [PATCH]exit.c: support larger exit code

[Zhang, Wei-Jovi (NSN - CN/Hangzhou)]

>Alexander Clouter <alex <at> digriz.org.uk> wrote:
>>Oleg Nesterov <oleg <at> redhat.com> wrote:
>>>
>>> Nowadays userspace application use systemcall exit/exit_group only
>>> support one byte exit code.
>>> In some cases this exit code range is too small for some "big
>>> application"(like telecom software, 255 is not enough).
>>>
>>> So we can give some "big application" a chance to get larger exit
code
>>> from child process.
>>> For other application don't want use larger exit code, they can use
>>> marco WEXITSTATUS to get lower one byte exit code.
>>>
>>> #define WEXITSTATUS(status)   __WEXITSTATUS (__WAIT_INT (status))
>>> --- stdlib.h
>>> #define       __WEXITSTATUS(status)   (((status) & 0xff00) >> 8)
>>> --- usrbits/waitstatus.h
>>>
>>>
>>> diff --git a/kernel/exit.c b/kernel/exit.c
>>> index ceffc67..8b13676 100644
>>> --- a/kernel/exit.c
>>> +++ b/kernel/exit.c
>>> @@ -1045,7 +1045,7 @@ EXPORT_SYMBOL(complete_and_exit);
>>>
>>>  SYSCALL_DEFINE1(exit, int, error_code)
>>>  {
>>> -       do_exit((error_code&0xff)<<8);
>>> +       do_exit(error_code << 8);
>>>  }
>>>
>>>  /*
>>> @@ -1086,7 +1086,7 @@ do_group_exit(int exit_code)
>>>   */
>>>  SYSCALL_DEFINE1(exit_group, int, error_code)
>>>  {
>>> -       do_group_exit((error_code & 0xff) << 8);
>>> +       do_group_exit(error_code << 8);
>>>         /* NOTREACHED */
>>>         return 0;
>>>  }
>> 
>> Hmm. Looking at this patch, I am wondering what was the reason for
the
>> current one-byte limitation.
>>

>The one byte limitation I think is all that is needed to give an 
>impression and *hint* of what went wrong, it was not ever meant to
cover 
>every possible error that the child process could report.  Even "small 
>programs" could generate $BIGNUM error codes it could be argued.

>Looking at the list for reserved exitcodes[1] everything covers 
>operational use and then if you look to /usr/include/sysexits.h for an 
>idea of the 'spirit' behind exitcodes, it is pretty clear it is all 
>rather 'generic' and vague to the precise reason, but it categorises 
>the type of error to maybe something the parent could gracefully
handle.  
>It is not a freetext communications channel :)

>If you have $BIGNUM outcomes of errors, you probably should not be
using 
>the exitcode path to communicate this information back up, although I 
>would agree it looks like very natural solution.  I would be more 
>inclined to pass up to the parent this information via a pipe (whether 
>that is via stdout, stderr or even a filesystem located pipe).  No
doubt 
>if you are trying to return $BIGNUM error codes, you probably want to 
>look more to an approach based on the one covered in RFC3463; there if 
>you return a sysexits.h type code then the child's STDOUT is consulted 
>for the extended error code reason...I think this approach is *very* 
>nice.


> The one byte limitation I think is all that is needed to give an
impression and *hint* of what went wrong, it was not ever meant to cover
every possible error that the child process could report.

I think this is just a convention of userspace program. if program want
to obey this convention, it should be use one-byte limition exit code, I
agree this.
so this mail thread's subject should change to "if program use exit code
with not want to obey the convention, kernel should return which value?"

so the key is:	from kernel point of view, kernel should don't care any
userspace program's convention.
how to use exit code is program's business, kernel should not to force
userspace program to obey some convention, it's not a good idea for
kernel.

if program use exit(43), kernel return 43; if program use exit(11111),
kernel should return 11111, not return 43. Apparently, program don't
want 43, it just want 11111. 
program is very hard to understand to get 43. program maybe don't
understand why she give kernel a int exit code, but kernel give her a
byte back. she don't know any convention, she just want program works.
So kernel can make more smooth for userspace program. 

This is my 2 cents. Pls give your comments freely. :-) 


.jovi

Re: [PATCH]exit.c: support larger exit code

[Alexander Clouter]

"Zhang, Wei-Jovi (NSN - CN/Hangzhou)" <wei-jovi.zhang@nsn.com> wrote:
> 
> I think this is just a convention of userspace program. if program 
> want to obey this convention, it should be use one-byte limition exit 
> code, I agree this. so this mail thread's subject should change to "if 
> program use exit code with not want to obey the convention, kernel 
> should return which value?"
>
I do have to agree with you on this point, although I stand by my 
thoughts on *how* you should be using the exit code.  Whilst writing my 
post I was thinking why in C is it 'int main()' and not 'u8 main()'.

A dig around with my Googlefu and on Wackipedia gave me nothing 
either...

One thing I can think of why the kernel is forcing a one byte return 
code is that:
 * guarantee there is no endian issue; hard to pull off though but I 
	guess that exit code could travel across IP to another 
	architecture
 * stop people abusing the exitcode :)

There is probably a deeper reason as errno.h/errno-base.h all seem to be 
one byte return codes too.  I'm starting to ponder if that top three 
bytes are meant to carry some other information?

Cheers

-- 
Alexander Clouter
.sigmonster says: Make sure your code does nothing gracefully.

Re: [PATCH]exit.c: support larger exit code

[Alan Cox]

> There is probably a deeper reason as errno.h/errno-base.h all seem to be 
> one byte return codes too.  I'm starting to ponder if that top three 
> bytes are meant to carry some other information?

man 2 wait

and also note that int was 16bits in the times when Unix wait/exit
behaviour was formulated.

Alan