Incorrect documentation for system, system*, and/or waitpid?

Incorrect documentation for system, system*, and/or waitpid?

[Chris Marusich]

[-- Attachment #1: Type: text/plain, Size: 2066 bytes --]

Hi,

I was reading about the "system" and "system*" procedures in the Guile
manual, and it seems like the actual behavior of Guile seems to
contradict what the manual says.  I'm using Guile version 2.2.4.

The Guile manual says that these procedures return the exit status as
returned by waitpid:

 -- Scheme Procedure: system [cmd]
 -- C Function: scm_system (cmd)
     Execute CMD using the operating system’s “command processor”.
     Under Unix this is usually the default shell ‘sh’.  The value
     returned is CMD’s exit status as returned by ‘waitpid’ [...].

 -- Scheme Procedure: system* arg1 arg2 ...
 -- C Function: scm_system_star (args)
     [...]

     This function returns the exit status of the command as provided by
     ‘waitpid’.  This value can be handled with ‘status:exit-val’ and
     the related functions.

     [...]

And the documentation for "waitpid" says:

 -- Scheme Procedure: waitpid pid [options]
 -- C Function: scm_waitpid (pid, options)
     [...]

     The return value is a pair containing:

       1. The process ID of the child process, or 0 if ‘WNOHANG’ was
          specified and no process was collected.
       2. The integer status value.

However, the return value doesn't appear to be a pair:

    scheme@(guile-user)> (system "true")
    $1 = 0
    scheme@(guile-user)> (pair? $1)
    $2 = #f
    scheme@(guile-user)> (system* "true")
    $3 = 0
    scheme@(guile-user)> (pair? $3)
    $4 = #f
    scheme@(guile-user)> 

In spite of this, the procedures like status:exit-val do seem to work as
expected:

    scheme@(guile-user)> (status:exit-val $1)
    $5 = 0
    scheme@(guile-user)> (status:exit-val $3)
    $6 = 0

Is the documentation incorrect?  I checked the Guile source for the
system procedure, and it seemed to me like the documentation is
incorrect, but I'm not 100% sure, since I'm not very familiar with the
Guile source code, so I thought I'd ask here to make sure.

Thank you for your help!

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: Incorrect documentation for system, system*, and/or waitpid?

[Mark H Weaver]

Hi Chris,

Chris Marusich <cmmarusich@gmail.com> writes:

> I was reading about the "system" and "system*" procedures in the Guile
> manual, and it seems like the actual behavior of Guile seems to
> contradict what the manual says.  I'm using Guile version 2.2.4.
>
> The Guile manual says that these procedures return the exit status as
> returned by waitpid:
>
>  -- Scheme Procedure: system [cmd]
>  -- C Function: scm_system (cmd)
>      Execute CMD using the operating system’s “command processor”.
>      Under Unix this is usually the default shell ‘sh’.  The value
>      returned is CMD’s exit status as returned by ‘waitpid’ [...].
>
>  -- Scheme Procedure: system* arg1 arg2 ...
>  -- C Function: scm_system_star (args)
>      [...]
>
>      This function returns the exit status of the command as provided by
>      ‘waitpid’.  This value can be handled with ‘status:exit-val’ and
>      the related functions.
>
>      [...]
>
> And the documentation for "waitpid" says:
>
>  -- Scheme Procedure: waitpid pid [options]
>  -- C Function: scm_waitpid (pid, options)
>      [...]
>
>      The return value is a pair containing:
>
>        1. The process ID of the child process, or 0 if ‘WNOHANG’ was
>           specified and no process was collected.
>        2. The integer status value.
>
> However, the return value doesn't appear to be a pair:

When the manual says "exit status as returned by ‘waitpid’", it's
referring to the "status value" portion of what 'waitpid' returns,
i.e. the CDR of 'waitpid's return value.

In other words, the manual is using the phrase "as returned by" in a
broader sense than your interpretation, merely meaning that it's among
the information returned by 'waitpid'.

This is based on the idea that 'waitpid' conceptually has two return
values, one of which is the status value.  If we designed this API
today, we would likely have it return two separate values, but 'waitpid'
predates the (somewhat controversial) addition of multiple return values
to Scheme in 1998, and so it returns the two values as a pair instead.

> In spite of this, the procedures like status:exit-val do seem to work as
> expected:
>
>     scheme@(guile-user)> (status:exit-val $1)
>     $5 = 0
>     scheme@(guile-user)> (status:exit-val $3)
>     $6 = 0

Right, these procedures are meant to operate on the status value.

> Is the documentation incorrect?

I'm not sure I'd call it "incorrect", but I agree that it's somewhat
confusing and could use clarification.  Would you like to propose a
patch?

    Regards,
      Mark

Re: Incorrect documentation for system, system*, and/or waitpid?

[Chris Marusich]

[-- Attachment #1.1: Type: text/plain, Size: 1263 bytes --]

Hi Mark!

Mark H Weaver <mhw@netris.org> writes:

> When the manual says "exit status as returned by ‘waitpid’", it's
> referring to the "status value" portion of what 'waitpid' returns,
> i.e. the CDR of 'waitpid's return value.

Thank you for the clarification!  It makes more sense now.

>>     scheme@(guile-user)> (status:exit-val $1)
>>     $5 = 0
>>     scheme@(guile-user)> (status:exit-val $3)
>>     $6 = 0
>
> Right, these procedures are meant to operate on the status value.

I see.  Then what's the intended use of status:exit-val?  I've read its
documentation and viewed its source a few times, and it seems like this
procedure basically behaves like the identity function.  I'm having
trouble thinking of a case where one would use status:exit-val instead
of simply using the integer status value directly.

>> Is the documentation incorrect?
>
> I'm not sure I'd call it "incorrect", but I agree that it's somewhat
> confusing and could use clarification.  Would you like to propose a
> patch?

I'm still a little confused about the intended use of status:exit-val,
but how does the attached patch look to you?  It's a small change, but I
think this would have been enough to dispel my confusion.

-- 
Chris

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Clarify-the-manual-s-Processes-section.patch --]
[-- Type: text/x-patch, Size: 998 bytes --]

From 4acd5ec212415efff6b9f847d1a7b9c6c9e7a526 Mon Sep 17 00:00:00 2001
From: Chris Marusich <cmmarusich@gmail.com>
Date: Sun, 14 Oct 2018 00:47:23 -0700
Subject: [PATCH] Clarify the manual's "Processes" section.

* doc/ref/posix.texi (Processes): Explicitly state that status:exit-val,
status:term-sig, and status:stop-sig take an integer as input.
---
 doc/ref/posix.texi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/ref/posix.texi b/doc/ref/posix.texi
index 5cb68a292..e39ea119c 100644
--- a/doc/ref/posix.texi
+++ b/doc/ref/posix.texi
@@ -1749,8 +1749,8 @@ The integer status value.
 @end deffn
 
 The following three
-functions can be used to decode the process status code returned
-by @code{waitpid}.
+functions take an integer as input and can be used to decode the integer
+status value returned by @code{waitpid}.
 
 @deffn {Scheme Procedure} status:exit-val status
 @deffnx {C Function} scm_status_exit_val (status)
-- 
2.18.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: Incorrect documentation for system, system*, and/or waitpid?

[Chris Vine]

On Sun, 14 Oct 2018 00:59:03 -0700
Chris Marusich <cmmarusich@gmail.com> wrote:
> Hi Mark!
> 
> Mark H Weaver <mhw@netris.org> writes:
> 
> > When the manual says "exit status as returned by ‘waitpid’", it's
> > referring to the "status value" portion of what 'waitpid' returns,
> > i.e. the CDR of 'waitpid's return value.
> 
> Thank you for the clarification!  It makes more sense now.
> 
> >>     scheme@(guile-user)> (status:exit-val $1)
> >>     $5 = 0
> >>     scheme@(guile-user)> (status:exit-val $3)
> >>     $6 = 0
> >
> > Right, these procedures are meant to operate on the status value.
> 
> I see.  Then what's the intended use of status:exit-val?  I've read its
> documentation and viewed its source a few times, and it seems like this
> procedure basically behaves like the identity function.  I'm having
> trouble thinking of a case where one would use status:exit-val instead
> of simply using the integer status value directly.

According to the documentation, status:exit-val returns #f if the
process did not end normally (that is, it did not end by main()
returning or by a call to exit()); otherwise if the process did end
normally it returns the process's exit status (that is, 0 if main()
returned otherwise the value passed to exit()).  It does this by
applying the WIFEXITED and WEXITSTATUS macros to POSIX waitpid()'s
wstatus out parameter (that out parameter being the cdr of guile's
waitpid procedure).  Note that the exit status provided by WEXITSTATUS
(and returned by status:exit-val where it doesn't return #f) is not
necessarily the integer comprising the wstatus out parameter. According
to POSIX it is in fact the lower 8 bits of that value.

So status:exit-val definitely is not an identity function.  The
documentation is correct, though perhaps it needs knowledge of POSIX
waitpid() to fully understand it.  Perhaps it would be better if "The
integer status value" was replaced by "The process status code" so the
same expression is used for it in both contexts, thus also helping
avoid confusion with "the exit status value" returned by
status:exit-val.

As "the process status code" is opaque, you don't really need to know
what is in it at all.  You just need to pass it to one or more of the
three procedures which decode it.

> >> Is the documentation incorrect?
> >
> > I'm not sure I'd call it "incorrect", but I agree that it's somewhat
> > confusing and could use clarification.  Would you like to propose a
> > patch?
> 
> I'm still a little confused about the intended use of status:exit-val,
> but how does the attached patch look to you?  It's a small change, but I
> think this would have been enough to dispel my confusion.

Re: Incorrect documentation for system, system*, and/or waitpid?

[Mark H Weaver]

Hi Chris,

Chris Marusich <cmmarusich@gmail.com> writes:

> Mark H Weaver <mhw@netris.org> writes:
>
>>>     scheme@(guile-user)> (status:exit-val $1)
>>>     $5 = 0
>>>     scheme@(guile-user)> (status:exit-val $3)
>>>     $6 = 0
>>
>> Right, these procedures are meant to operate on the status value.
>
> I see.  Then what's the intended use of status:exit-val?  I've read its
> documentation and viewed its source a few times, and it seems like this
> procedure basically behaves like the identity function.

I'm not sure where you got that idea, but it's definitely not the
identity function.  It uses standard POSIX macros from <sys/wait.h> to
interpret the status value and extract information from bit fields
within.  The details of how the status value is represented are platform
specific, but on GNU/Linux, in the case where the program exited
normally, the low 7 bits are zeroes and bits 8-15 contain the program's
exit value.  Other representations are used when the program was
terminated or stopped by a signal.  See the macro definitions in
<sys/wait.h> and <bits/waitstatus.h> for the full details.

> I'm having
> trouble thinking of a case where one would use status:exit-val instead
> of simply using the integer status value directly.

There's one case where you can use the integer status value directly:
you can check if it's zero, and if it is, that indicates that the
program exited normally with zero exit value.

If the status value is non-zero, and you want to know more information
about what happened, then you must use the status:* procedures.

>>> Is the documentation incorrect?
>>
>> I'm not sure I'd call it "incorrect", but I agree that it's somewhat
>> confusing and could use clarification.  Would you like to propose a
>> patch?
>
> I'm still a little confused about the intended use of status:exit-val,
> but how does the attached patch look to you?  It's a small change, but I
> think this would have been enough to dispel my confusion.
[...]
> diff --git a/doc/ref/posix.texi b/doc/ref/posix.texi
> index 5cb68a292..e39ea119c 100644
> --- a/doc/ref/posix.texi
> +++ b/doc/ref/posix.texi
> @@ -1749,8 +1749,8 @@ The integer status value.
>  @end deffn
>  
>  The following three
> -functions can be used to decode the process status code returned
> -by @code{waitpid}.
> +functions take an integer as input and can be used to decode the integer
> +status value returned by @code{waitpid}.
>  
>  @deffn {Scheme Procedure} status:exit-val status
>  @deffnx {C Function} scm_status_exit_val (status)

It sounds a bit repetitive to me.  I wonder if it would be enough to
simply change "process status code" to "integer status value", since
those are the same three words used in the immediately preceding
description of waitpid's return value, and it also clarifies that it's
an integer.

What do you think?

    Thanks,
      Mark

Re: Incorrect documentation for system, system*, and/or waitpid?

[Chris Marusich]

[-- Attachment #1.1: Type: text/plain, Size: 1293 bytes --]

Hi Mark,

Thank you for the reply!

Mark H Weaver <mhw@netris.org> writes:

>> I see.  Then what's the intended use of status:exit-val?  I've read its
>> documentation and viewed its source a few times, and it seems like this
>> procedure basically behaves like the identity function.
>
> I'm not sure where you got that idea, but it's definitely not the
> identity function.

I didn't realize that additional information was encoded in the integer
status value.  That explains why I was confused.

>> I'm having trouble thinking of a case where one would use
>> status:exit-val instead of simply using the integer status value
>> directly.
>
> There's one case where you can use the integer status value directly:
> you can check if it's zero, and if it is, that indicates that the
> program exited normally with zero exit value.
>
> If the status value is non-zero, and you want to know more information
> about what happened, then you must use the status:* procedures.

This makes sense now.

> [...] I wonder if it would be enough to simply change "process status
> code" to "integer status value" [...].

I agree.  I've attached a new patch that makes the change.  It also adds
a cross-reference that I think is helpful.  How does it look?

-- 
Chris

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Clarify-the-manual-s-Processes-section.patch --]
[-- Type: text/x-patch, Size: 1296 bytes --]

From 720a2069286e983b63b2d66b4471bc4e30a8d2cc Mon Sep 17 00:00:00 2001
From: Chris Marusich <cmmarusich@gmail.com>
Date: Sun, 14 Oct 2018 00:47:23 -0700
Subject: [PATCH] Clarify the manual's "Processes" section.

* doc/ref/posix.texi (Processes): Use the phrase "integer status value"
consistently, and add a cross-reference to the section of the glibc
manual that explains what it is.
---
 doc/ref/posix.texi | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/doc/ref/posix.texi b/doc/ref/posix.texi
index 5cb68a292..388efc368 100644
--- a/doc/ref/posix.texi
+++ b/doc/ref/posix.texi
@@ -1744,13 +1744,14 @@ The return value is a pair containing:
 The process ID of the child process, or 0 if @code{WNOHANG} was
 specified and no process was collected.
 @item
-The integer status value.
+The integer status value (@pxref{Process Completion Status,,, libc, The
+GNU C Library Reference Manual}).
 @end enumerate
 @end deffn
 
 The following three
-functions can be used to decode the process status code returned
-by @code{waitpid}.
+functions can be used to decode the integer status value returned by
+@code{waitpid}.
 
 @deffn {Scheme Procedure} status:exit-val status
 @deffnx {C Function} scm_status_exit_val (status)
-- 
2.19.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: Incorrect documentation for system, system*, and/or waitpid?

[Mark H Weaver]

Hi Chris,

Chris Marusich <cmmarusich@gmail.com> writes:

> Mark H Weaver <mhw@netris.org> writes:
>
>> [...] I wonder if it would be enough to simply change "process status
>> code" to "integer status value" [...].
>
> I agree.  I've attached a new patch that makes the change.  It also adds
> a cross-reference that I think is helpful.  How does it look?

Looks good to me.  I pushed it to the stable-2.2 branch.

     Thanks!
       Mark

Re: Incorrect documentation for system, system*, and/or waitpid?

[Chris Marusich]

[-- Attachment #1: Type: text/plain, Size: 159 bytes --]

Hi Mark,

Mark H Weaver <mhw@netris.org> writes:

> Looks good to me.  I pushed it to the stable-2.2 branch.

Great, thank you very much!

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]