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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[[ 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@xxxxxxxxx> 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@xxxxxxxxxx>.
> 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/



[Index of Archives]     [Kernel Documentation]     [Netdev]     [Linux Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux