Re: ptrace.2: Simplify signature? s/enum \w*/int/

[[ CC += Denys, glibc ]]

Hi Michael,

On 2/11/21 7:50 AM, Michael Kerrisk (man-pages) wrote:

Hi Alex,

quoted

On Tue, Feb 9, 2021, 23:34 Alejandro Colomar (man-pages) <
alx.manpages@gmail.com> wrote:

quoted

Hi Michael,

On 2/9/21 7:25 PM, Michael Kerrisk (man-pages) wrote:

quoted

Hi Alex,

On 2/8/21 11:36 PM, Alejandro Colomar (man-pages) wrote:

quoted

[CC += linux-man@]

I forgot the list.

On 2/8/21 11:34 PM, Alejandro Colomar (man-pages) wrote:

quoted

Hi Michael,

I think we should simplify the prototype of ptrace(2) from using 'enum
__ptrace_request' to 'int'.  It is an implementation detail that should
be transparent to the user.  Other pages where glibc uses an 'enum' are
documented to use 'int' (I don't remember the names of those, but
remember having seen them a few days ago.  Otherwise, we might have to
document 'enum' elsewhere, which I don't think adds any value.  What do
you think about it?

I'm somewhat conservative on this point. It's been documented
this way forever, so I'm inclined to pause before changing it.

Okay.

quoted

I feel like we lack information. I'd like to know about some of
the other cases where enums in glibc are documented instead as int.
But, on the other hand, maybe this is not the highest priority,
so it may not be worth too much effort to discover those examples.

I checked all of them ;)
It's more like 50/50.

Here are the glibc results (grepping though the kernel is taking much
longer, but I guess the command will have ended by tomorrow morning, and
I'll send you a follow-up.


.../gnu/glibc$ man_lsfunc ../../linux/man-pages/man2 \
               |while read -r f; do \
                       grep_glibc_prototype $f;
               done \
               |grep enum;
extern long pciconfig_iobase(enum __pciconfig_iobase_which __which,
extern int prlimit (__pid_t __pid, enum __rlimit_resource __resource,
extern int prlimit (__pid_t __pid, enum __rlimit_resource __resource,
extern int prlimit (__pid_t __pid, enum __rlimit_resource __resource,
extern int prlimit (__pid_t __pid, enum __rlimit_resource __resource,
extern int ptrace (enum __ptrace_request __request, ...);
extern long int ptrace (enum __ptrace_request __request, ...) __THROW;
.../gnu/glibc$


Of the above, only ptrace(2) uses 'enum' in the manual page.
The others use 'int'.


.../gnu/glibc$ man_lsfunc ../../linux/man-pages/man3 \
               |while read -r f; do \
                       grep_glibc_prototype $f;
               done \
               |grep enum;
extern int mcheck (void (*__abortfunc)(enum mcheck_status)) __THROW;
extern int mcheck_pedantic (void (*__abortfunc)(enum mcheck_status))
__THROW;
extern enum mcheck_status mprobe (void *__ptr) __THROW;
.../gnu/glibc$

All of the above use 'enum' in the manual page.

Cheers,

Alex

......

man_lsfunc prints a list of all C functions documented in the SYNOPSIS
of a directory such as man2 (it can also be used on a single manual
page, such as `man_lsfunc man2/open.2`).

function man_lsfunc()
{
        if ! [ -v 1 ]; then
                >&2 echo "Usage: ${FUNCNAME[0]} <dir>";
                return ${EX_USAGE};
        fi

        find "${1}" -type f \
        |xargs grep -l "\.SH SYNOPSIS" \
        |sort -V \
        |while read -r manpage; do
                <${manpage} \
                sed -n \
                        -e '/^\.TH/,/^\.SH/{/^\.SH/!p}' \
                        -e "/^\.SH SYNOPSIS/p" \
                        -e "/^\.SH SYNOPSIS/,/^\.SH/{/^\.SH/!p}" \
                |sed \
                        -e '/Feature/,$d' \
                        -e '/:/,$d' \
                |man -P cat -l - 2>/dev/null;
        done \
        |sed -n "/^SYNOPSIS/,/^\w/p" \
        |grep '^       \w' \
        |grep -v '[{}]' \
        |sed 's/^[^(]* \**\(\w*\)(.*/\1/' \
        |grep '^\w' \
        |sort \
        |uniq;
}

quoted

Thanks,

Michael

On 2/10/21 10:55 PM, Alejandro Colomar wrote:

quoted

[offlist; phone]

Hi Michael,

I'd like to make sure you also read this email.

Yes, I saw it. I'll go with your judgement on this one. I do not 
have strong feelings about it.

If you send the patch, please CC Denys Vlasenko [off-list ref].
Denys has made many contributions to the ptrace.2 page in
the past, and just maybe he has a comment.

Thanks,

Michael

(I fixed my top reply.)


I found that there are a few more functions using enum.
I didn't find them before, because they're probably macros?.

[[
$ man_section man2 SYNOPSIS | grep enum;
       long ptrace(enum __ptrace_request request, pid_t pid,
$ man_section man3 SYNOPSIS | grep '\benum\b'
       int mcheck(void (*abortfunc)(enum mcheck_status mstatus));
       int mcheck_pedantic(void (*abortfunc)(enum mcheck_status mstatus));
       enum mcheck_status mprobe(void *ptr);
              of enum clnt_stat cast to an integer if it fails.  The
routine clnt_perrno()
       enum clnt_stat clnt_broadcast(unsigned long prognum,
       enum clnt_stat clnt_call(CLIENT *clnt, unsigned long procnum,
       void clnt_perrno(enum clnt_stat stat);
       char *clnt_sperrno(enum clnt_stat stat);
       enum clnt_stat pmap_rmtcall(struct sockaddr_in *addr,
       void svcerr_auth(SVCXPRT *xprt, enum auth_stat why);
       typedef enum { preorder, postorder, endorder, leaf } VISIT;
                          enum xdr_op op);
       void xdrstdio_create(XDR *xdrs, FILE *file, enum xdr_op op);
]]

I see a difference between the syscall wrappers and the library calls:
The syscall wrappers use uglified enum tags (__xxx), which I guess means
that they are being used as an implementation detail that were not
supposed to be known by the users.  But the library calls (man3)
deliberately use normal enum tags.  Also, the glibc documentation uses
enum for mcheck*(3).

The BSDs use int for the ptrace(2) request (both manual and implementation).

So I'd fix ptrace(2) to use 'int', to be consistent with the
documentation for other syscall wrappers (pciconfig_iobase(2),
prlimit(2)); and leave the section 3 functions untouched.  However, I'm
not really sure about this now, so I'll send this email, wait some time
to see if someone else comments, and then I'll see.

Thanks,

Alex


-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/

On 2/11/21 6:28 PM, Alejandro Colomar (man-pages) wrote:

[[ CC += Denys, glibc ]]

Hi Michael,

On 2/11/21 7:50 AM, Michael Kerrisk (man-pages) wrote:

quoted

Hi Alex,

quoted

On Tue, Feb 9, 2021, 23:34 Alejandro Colomar (man-pages) <
alx.manpages@gmail.com> wrote:

quoted

Hi Michael,

On 2/9/21 7:25 PM, Michael Kerrisk (man-pages) wrote:

quoted

Hi Alex,

On 2/8/21 11:36 PM, Alejandro Colomar (man-pages) wrote:

quoted

[CC += linux-man@]

I forgot the list.

On 2/8/21 11:34 PM, Alejandro Colomar (man-pages) wrote:

quoted

Hi Michael,

I think we should simplify the prototype of ptrace(2) from using 'enum
__ptrace_request' to 'int'.  It is an implementation detail that should
be transparent to the user.  Other pages where glibc uses an 'enum' are
documented to use 'int' (I don't remember the names of those, but
remember having seen them a few days ago.  Otherwise, we might have to
document 'enum' elsewhere, which I don't think adds any value.  What do
you think about it?

I'm somewhat conservative on this point. It's been documented
this way forever, so I'm inclined to pause before changing it.

Okay.

quoted

I feel like we lack information. I'd like to know about some of
the other cases where enums in glibc are documented instead as int.
But, on the other hand, maybe this is not the highest priority,
so it may not be worth too much effort to discover those examples.

I checked all of them ;)
It's more like 50/50.

Here are the glibc results (grepping though the kernel is taking much
longer, but I guess the command will have ended by tomorrow morning, and
I'll send you a follow-up.


.../gnu/glibc$ man_lsfunc ../../linux/man-pages/man2 \
               |while read -r f; do \
                       grep_glibc_prototype $f;
               done \
               |grep enum;
extern long pciconfig_iobase(enum __pciconfig_iobase_which __which,
extern int prlimit (__pid_t __pid, enum __rlimit_resource __resource,
extern int prlimit (__pid_t __pid, enum __rlimit_resource __resource,
extern int prlimit (__pid_t __pid, enum __rlimit_resource __resource,
extern int prlimit (__pid_t __pid, enum __rlimit_resource __resource,
extern int ptrace (enum __ptrace_request __request, ...);
extern long int ptrace (enum __ptrace_request __request, ...) __THROW;
.../gnu/glibc$


Of the above, only ptrace(2) uses 'enum' in the manual page.
The others use 'int'.


.../gnu/glibc$ man_lsfunc ../../linux/man-pages/man3 \
               |while read -r f; do \
                       grep_glibc_prototype $f;
               done \
               |grep enum;
extern int mcheck (void (*__abortfunc)(enum mcheck_status)) __THROW;
extern int mcheck_pedantic (void (*__abortfunc)(enum mcheck_status))
__THROW;
extern enum mcheck_status mprobe (void *__ptr) __THROW;
.../gnu/glibc$

All of the above use 'enum' in the manual page.

Cheers,

Alex

......

man_lsfunc prints a list of all C functions documented in the SYNOPSIS
of a directory such as man2 (it can also be used on a single manual
page, such as `man_lsfunc man2/open.2`).

function man_lsfunc()
{
        if ! [ -v 1 ]; then
                >&2 echo "Usage: ${FUNCNAME[0]} <dir>";
                return ${EX_USAGE};
        fi

        find "${1}" -type f \
        |xargs grep -l "\.SH SYNOPSIS" \
        |sort -V \
        |while read -r manpage; do
                <${manpage} \
                sed -n \
                        -e '/^\.TH/,/^\.SH/{/^\.SH/!p}' \
                        -e "/^\.SH SYNOPSIS/p" \
                        -e "/^\.SH SYNOPSIS/,/^\.SH/{/^\.SH/!p}" \
                |sed \
                        -e '/Feature/,$d' \
                        -e '/:/,$d' \
                |man -P cat -l - 2>/dev/null;
        done \
        |sed -n "/^SYNOPSIS/,/^\w/p" \
        |grep '^       \w' \
        |grep -v '[{}]' \
        |sed 's/^[^(]* \**\(\w*\)(.*/\1/' \
        |grep '^\w' \
        |sort \
        |uniq;
}

quoted

Thanks,

Michael

On 2/10/21 10:55 PM, Alejandro Colomar wrote:

quoted

[offlist; phone]

Hi Michael,

I'd like to make sure you also read this email.

Yes, I saw it. I'll go with your judgement on this one. I do not 
have strong feelings about it.

If you send the patch, please CC Denys Vlasenko [off-list ref].
Denys has made many contributions to the ptrace.2 page in
the past, and just maybe he has a comment.

Thanks,

Michael

(I fixed my top reply.)


I found that there are a few more functions using enum.
I didn't find them before, because they're probably macros?.

Hi Michael,

And a few more (implemented with 'enum', documented with 'int'):

=============================  getitimer
time/sys/time.h:123:
int getitimer (itimer_which_t which,
                      struct itimerval *value) THROW;
=============================  setitimer
time/sys/time.h:129:
int setitimer (itimer_which_t which,
                      const struct itimerval *restrict new,
                      struct itimerval *restrict old) THROW;


I think there might be a few more like these, which use a 'typedef'd
'enum'.  POSIX specifies 'int' for them.

Cheers,

Alex

[[
$ man_section man2 SYNOPSIS | grep enum;
       long ptrace(enum __ptrace_request request, pid_t pid,
$ man_section man3 SYNOPSIS | grep '\benum\b'
       int mcheck(void (*abortfunc)(enum mcheck_status mstatus));
       int mcheck_pedantic(void (*abortfunc)(enum mcheck_status mstatus));
       enum mcheck_status mprobe(void *ptr);
              of enum clnt_stat cast to an integer if it fails.  The
routine clnt_perrno()
       enum clnt_stat clnt_broadcast(unsigned long prognum,
       enum clnt_stat clnt_call(CLIENT *clnt, unsigned long procnum,
       void clnt_perrno(enum clnt_stat stat);
       char *clnt_sperrno(enum clnt_stat stat);
       enum clnt_stat pmap_rmtcall(struct sockaddr_in *addr,
       void svcerr_auth(SVCXPRT *xprt, enum auth_stat why);
       typedef enum { preorder, postorder, endorder, leaf } VISIT;
                          enum xdr_op op);
       void xdrstdio_create(XDR *xdrs, FILE *file, enum xdr_op op);
]]

I see a difference between the syscall wrappers and the library calls:
The syscall wrappers use uglified enum tags (__xxx), which I guess means
that they are being used as an implementation detail that were not
supposed to be known by the users.  But the library calls (man3)
deliberately use normal enum tags.  Also, the glibc documentation uses
enum for mcheck*(3).

The BSDs use int for the ptrace(2) request (both manual and implementation).

So I'd fix ptrace(2) to use 'int', to be consistent with the
documentation for other syscall wrappers (pciconfig_iobase(2),
prlimit(2)); and leave the section 3 functions untouched.  However, I'm
not really sure about this now, so I'll send this email, wait some time
to see if someone else comments, and then I'll see.

Thanks,

Alex

-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/