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

["Alejandro Colomar (man-pages)" <alx.manpages@gmail.com>]

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:
>> Hi Alex,
>>
>>> On Tue, Feb 9, 2021, 23:34 Alejandro Colomar (man-pages) <
>>> alx.manpages@gmail.com> wrote:
>>>
>>>> Hi Michael,
>>>>
>>>> On 2/9/21 7:25 PM, Michael Kerrisk (man-pages) wrote:
>>>>> Hi Alex,
>>>>>
>>>>> On 2/8/21 11:36 PM, Alejandro Colomar (man-pages) wrote:
>>>>>> [CC += linux-man@]
>>>>>>
>>>>>> I forgot the list.
>>>>>>
>>>>>> On 2/8/21 11:34 PM, Alejandro Colomar (man-pages) wrote:
>>>>>>> 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.
>>>>
>>>>> 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;
>>>> }
>>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Michael
>>>>>
>>>>
>>>
>>
>> On 2/10/21 10:55 PM, Alejandro Colomar wrote:
>>> [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 <dvlasenk@redhat.com>.
>> 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/