diff mbox series

[man-pages,v3,2/4] madvise.2: document reliable probe for advice support

Message ID 20221021223300.3675201-3-zokeefe@google.com (mailing list archive)
State New
Headers show
Series Add MADV_COLLAPSE documentation | expand

Commit Message

Zach O'Keefe Oct. 21, 2022, 10:32 p.m. UTC
From: Zach O'Keefe <zokeefe@google.com>

EINVAL is an overloaded error code for madvise(2) and it's not clear
under what context it means "advice is not valid" vs another error.

Explicitly document that madvise(0, 0, advice) can reliably be used to
probe for kernel support for "advice", returning zero iff "advice" is
supported by the kernel.

Signed-off-by: Zach O'Keefe <zokeefe@google.com>
---
 man2/madvise.2 | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Alejandro Colomar Oct. 30, 2022, 11:44 a.m. UTC | #1
Hi Zach,

On 10/22/22 00:32, Zach OKeefe wrote:
> From: Zach O'Keefe <zokeefe@google.com>
> 
> EINVAL is an overloaded error code for madvise(2) and it's not clear
> under what context it means "advice is not valid" vs another error.
> 
> Explicitly document that madvise(0, 0, advice) can reliably be used to
> probe for kernel support for "advice", returning zero iff "advice" is
> supported by the kernel.
> 
> Signed-off-by: Zach O'Keefe <zokeefe@google.com>

Patch applied.

Thanks,

Alex

> ---
>   man2/madvise.2 | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/man2/madvise.2 b/man2/madvise.2
> index 64f788ace..df3413cc8 100644
> --- a/man2/madvise.2
> +++ b/man2/madvise.2
> @@ -790,6 +790,11 @@ that are not mapped, the Linux version of
>   ignores them and applies the call to the rest (but returns
>   .B ENOMEM
>   from the system call, as it should).
> +.PP
> +.IR madvise(0,\ 0,\ advice)
> +will return zero iff
> +.I advice
> +is supported by the kernel and can be relied on to probe for support.
>   .\" .SH HISTORY
>   .\" The
>   .\" .BR madvise ()
Zach O'Keefe Oct. 31, 2022, 4:33 p.m. UTC | #2
On Sun, Oct 30, 2022 at 4:44 AM Alejandro Colomar
<alx.manpages@gmail.com> wrote:
>
> Hi Zach,
>
> On 10/22/22 00:32, Zach OKeefe wrote:
> > From: Zach O'Keefe <zokeefe@google.com>
> >
> > EINVAL is an overloaded error code for madvise(2) and it's not clear
> > under what context it means "advice is not valid" vs another error.
> >
> > Explicitly document that madvise(0, 0, advice) can reliably be used to
> > probe for kernel support for "advice", returning zero iff "advice" is
> > supported by the kernel.
> >
> > Signed-off-by: Zach O'Keefe <zokeefe@google.com>
>
> Patch applied.

Thank you!

Best,
Zach

> Thanks,
>
> Alex
>
> > ---
> >   man2/madvise.2 | 5 +++++
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/man2/madvise.2 b/man2/madvise.2
> > index 64f788ace..df3413cc8 100644
> > --- a/man2/madvise.2
> > +++ b/man2/madvise.2
> > @@ -790,6 +790,11 @@ that are not mapped, the Linux version of
> >   ignores them and applies the call to the rest (but returns
> >   .B ENOMEM
> >   from the system call, as it should).
> > +.PP
> > +.IR madvise(0,\ 0,\ advice)
> > +will return zero iff
> > +.I advice
> > +is supported by the kernel and can be relied on to probe for support.
> >   .\" .SH HISTORY
> >   .\" The
> >   .\" .BR madvise ()
>
> --
> <http://www.alejandro-colomar.es/>
Alejandro Colomar Oct. 31, 2022, 8:21 p.m. UTC | #3
Hi Zach,

On 10/31/22 17:33, Zach O'Keefe wrote:
>> Patch applied.
> 
> Thank you!
> 
> Best,
> Zach
> 
>> Thanks,
>>
>> Alex
>>

I just caught a small formatting issue by running some linters through that file:

$ make -ij lint >/dev/null 2>&1
$ touch man2/madvise.2
$ make -k lint-man
LINT (groff)	tmp/lint/man2/madvise.2.lint-man.groff.touch
an.tmac:man2/madvise.2:795: style: .IR expects at least 2 arguments, got 1
found style problems; aborting
make: *** [lib/lint-man.mk:77: tmp/lint/man2/madvise.2.lint-man.groff.touch] Error 1
LINT (mandoc)	tmp/lint/man2/madvise.2.lint-man.mandoc.touch
mandoc: man2/madvise.2:15:5: STYLE: lower case character in document title: TH 
madvise
mandoc: man2/madvise.2:15:15: WARNING: cannot parse date, using it verbatim: TH 
(date)
make: *** [lib/lint-man.mk:87: tmp/lint/man2/madvise.2.lint-man.mandoc.touch] 
Error 2
make: Target 'lint-man' not remade because of errors.


The issues reported by mandoc(1) are expected; but the issue reported by 
groff(1) is legit.  The offending line is:

$ sed -n 795p man2/madvise.2
.IR madvise(0,\ 0,\ advice)

Since all of the line is formatted in italics (no roman part), it should be .I 
and not .IR.
After fixing that line, the linter now says:

$ make lint-man -k
LINT (groff)	tmp/lint/man2/madvise.2.lint-man.groff.touch
LINT (mandoc)	tmp/lint/man2/madvise.2.lint-man.mandoc.touch
mandoc: man2/madvise.2:15:5: STYLE: lower case character in document title: TH 
madvise
mandoc: man2/madvise.2:15:15: WARNING: cannot parse date, using it verbatim: TH 
(date)
make: *** [lib/lint-man.mk:87: tmp/lint/man2/madvise.2.lint-man.mandoc.touch] 
Error 2
make: Target 'lint-man' not remade because of errors.


Ignoring the spurious mandoc(1) warnings, it's all good.

Documentation about how to use this feature is here (written today, so no way 
you could have run it; I should have, though :D):
<https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/CONTRIBUTING#n133>

Cheers,

Alex

>>> ---
>>>    man2/madvise.2 | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/man2/madvise.2 b/man2/madvise.2
>>> index 64f788ace..df3413cc8 100644
>>> --- a/man2/madvise.2
>>> +++ b/man2/madvise.2
>>> @@ -790,6 +790,11 @@ that are not mapped, the Linux version of
>>>    ignores them and applies the call to the rest (but returns
>>>    .B ENOMEM
>>>    from the system call, as it should).
>>> +.PP
>>> +.IR madvise(0,\ 0,\ advice)
>>> +will return zero iff
>>> +.I advice
>>> +is supported by the kernel and can be relied on to probe for support.
>>>    .\" .SH HISTORY
>>>    .\" The
>>>    .\" .BR madvise ()
>>
>> --
>> <http://www.alejandro-colomar.es/>
Zach O'Keefe Oct. 31, 2022, 9:24 p.m. UTC | #4
Hey Alex,

On Mon, Oct 31, 2022 at 1:21 PM Alejandro Colomar
<alx.manpages@gmail.com> wrote:
>
> Hi Zach,
>
> On 10/31/22 17:33, Zach O'Keefe wrote:
> >> Patch applied.
> >
> > Thank you!
> >
> > Best,
> > Zach
> >
> >> Thanks,
> >>
> >> Alex
> >>
>
> I just caught a small formatting issue by running some linters through that file:
>
> $ make -ij lint >/dev/null 2>&1
> $ touch man2/madvise.2
> $ make -k lint-man
> LINT (groff)    tmp/lint/man2/madvise.2.lint-man.groff.touch
> an.tmac:man2/madvise.2:795: style: .IR expects at least 2 arguments, got 1
> found style problems; aborting
> make: *** [lib/lint-man.mk:77: tmp/lint/man2/madvise.2.lint-man.groff.touch] Error 1
> LINT (mandoc)   tmp/lint/man2/madvise.2.lint-man.mandoc.touch
> mandoc: man2/madvise.2:15:5: STYLE: lower case character in document title: TH
> madvise
> mandoc: man2/madvise.2:15:15: WARNING: cannot parse date, using it verbatim: TH
> (date)
> make: *** [lib/lint-man.mk:87: tmp/lint/man2/madvise.2.lint-man.mandoc.touch]
> Error 2
> make: Target 'lint-man' not remade because of errors.
>
>
> The issues reported by mandoc(1) are expected; but the issue reported by
> groff(1) is legit.  The offending line is:
>
> $ sed -n 795p man2/madvise.2
> .IR madvise(0,\ 0,\ advice)
>
> Since all of the line is formatted in italics (no roman part), it should be .I
> and not .IR.
> After fixing that line, the linter now says:
>
> $ make lint-man -k
> LINT (groff)    tmp/lint/man2/madvise.2.lint-man.groff.touch
> LINT (mandoc)   tmp/lint/man2/madvise.2.lint-man.mandoc.touch
> mandoc: man2/madvise.2:15:5: STYLE: lower case character in document title: TH
> madvise
> mandoc: man2/madvise.2:15:15: WARNING: cannot parse date, using it verbatim: TH
> (date)
> make: *** [lib/lint-man.mk:87: tmp/lint/man2/madvise.2.lint-man.mandoc.touch]
> Error 2
> make: Target 'lint-man' not remade because of errors.
>
>
> Ignoring the spurious mandoc(1) warnings, it's all good.
>
> Documentation about how to use this feature is here (written today, so no way
> you could have run it; I should have, though :D):
> <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/CONTRIBUTING#n133>
>

Thanks for taking a look here and thanks for adding the documentation
to CONTRIBUTING -- was easy to follow.

As you mentioned in 1/4, I don't have the groff(1) version (GNU groff
version 1.22.4) to see the error, but it seems like a real fix (& I've
taken a look at your fix patch - thank you :) )

Thanks again,

Best,
Zach
diff mbox series

Patch

diff --git a/man2/madvise.2 b/man2/madvise.2
index 64f788ace..df3413cc8 100644
--- a/man2/madvise.2
+++ b/man2/madvise.2
@@ -790,6 +790,11 @@  that are not mapped, the Linux version of
 ignores them and applies the call to the rest (but returns
 .B ENOMEM
 from the system call, as it should).
+.PP
+.IR madvise(0,\ 0,\ advice)
+will return zero iff
+.I advice
+is supported by the kernel and can be relied on to probe for support.
 .\" .SH HISTORY
 .\" The
 .\" .BR madvise ()