[PATCH v2] close_range.2: Add _GNU_SOURCE and unistd.h to SYNOPSIS

[PATCH v2] close_range.2: Add _GNU_SOURCE and unistd.h to SYNOPSIS

[Mark Wielaard]

close_range is defined in unistd.h when _GNU_SOURCE is defined.
The linux/close_range.h include file only defines the (linux specific)
flags constants. The flags argument is an int, not an unsigned int, in
the glibc wrapper. Use the close_range library call in the example code
instead of syscall.

Fixes: 71a62d6c3c56 ("close_range.2: Glibc added a wrapper recently")
Fixes: c2356ba085ed ("close_range.2: Glibc 2.34 has added a close_range() wrapper")

Reported-by: Alexandra Hájková <ahajkova@redhat.com>
Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 man2/close_range.2 | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/man2/close_range.2 b/man2/close_range.2
index 380a47365..62b728e96 100644
--- a/man2/close_range.2
+++ b/man2/close_range.2
@@ -11,10 +11,13 @@ Standard C library
 .RI ( libc ", " \-lc )
 .SH SYNOPSIS
 .nf
-.B #include <linux/close_range.h>
+.BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
+.B #include <unistd.h>
+.P
+.BR "#include <linux/close_range.h>" "  /* For the flags constants */"
 .P
 .BI "int close_range(unsigned int " first ", unsigned int " last ,
-.BI "                unsigned int " flags );
+.BI "                int " flags );
 .fi
 .SH DESCRIPTION
 The
@@ -205,7 +208,6 @@ result from the calls to
 #include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <sys/syscall.h>
 #include <unistd.h>
 \&
 /* Show the contents of the symbolic links in /proc/self/fd */
@@ -259,7 +261,7 @@ main(int argc, char *argv[])
 \&
     printf("========= About to call close_range() =======\en");
 \&
-    if (syscall(SYS_close_range, 3, \[ti]0U, 0) == \-1) {
+    if (close_range(3, \[ti]0U, 0) == \-1) {
         perror("close_range");
         exit(EXIT_FAILURE);
     }
-- 
2.39.3

Re: [PATCH v2] close_range.2: Add _GNU_SOURCE and unistd.h to SYNOPSIS

[Alejandro Colomar]

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

Hi Mark,

On Mon, Feb 12, 2024 at 12:29:03AM +0100, Mark Wielaard wrote:
> close_range is defined in unistd.h when _GNU_SOURCE is defined.
> The linux/close_range.h include file only defines the (linux specific)
> flags constants. The flags argument is an int, not an unsigned int, in
> the glibc wrapper. Use the close_range library call in the example code
> instead of syscall.
> 
> Fixes: 71a62d6c3c56 ("close_range.2: Glibc added a wrapper recently")
> Fixes: c2356ba085ed ("close_range.2: Glibc 2.34 has added a close_range() wrapper")
> 
> Reported-by: Alexandra Hájková <ahajkova@redhat.com>
> Signed-off-by: Mark Wielaard <mark@klomp.org>

Thanks!  LGTM, but please check a small comment below.

> ---
>  man2/close_range.2 | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/man2/close_range.2 b/man2/close_range.2
> index 380a47365..62b728e96 100644
> --- a/man2/close_range.2
> +++ b/man2/close_range.2
> @@ -11,10 +11,13 @@ Standard C library
>  .RI ( libc ", " \-lc )
>  .SH SYNOPSIS
>  .nf
> -.B #include <linux/close_range.h>
> +.BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
> +.B #include <unistd.h>
> +.P
> +.BR "#include <linux/close_range.h>" "  /* For the flags constants */"

Could you please format the comment like other pages that do the same
thing?  See for example membarrier(2).

Have a lovely night!
Alex

>  .P
>  .BI "int close_range(unsigned int " first ", unsigned int " last ,
> -.BI "                unsigned int " flags );
> +.BI "                int " flags );
>  .fi
>  .SH DESCRIPTION
>  The
> @@ -205,7 +208,6 @@ result from the calls to
>  #include <limits.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> -#include <sys/syscall.h>
>  #include <unistd.h>
>  \&
>  /* Show the contents of the symbolic links in /proc/self/fd */
> @@ -259,7 +261,7 @@ main(int argc, char *argv[])
>  \&
>      printf("========= About to call close_range() =======\en");
>  \&
> -    if (syscall(SYS_close_range, 3, \[ti]0U, 0) == \-1) {
> +    if (close_range(3, \[ti]0U, 0) == \-1) {
>          perror("close_range");
>          exit(EXIT_FAILURE);
>      }
> -- 
> 2.39.3
> 
> 

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

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

Re: [PATCH v2] close_range.2: Add _GNU_SOURCE and unistd.h to SYNOPSIS

[Mark Wielaard]

Hi Alejandro,

On Mon, Feb 12, 2024 at 12:49:32AM +0100, Alejandro Colomar wrote:
> Thanks!  LGTM, but please check a small comment below.
> 
> > ---
> >  man2/close_range.2 | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/man2/close_range.2 b/man2/close_range.2
> > index 380a47365..62b728e96 100644
> > --- a/man2/close_range.2
> > +++ b/man2/close_range.2
> > @@ -11,10 +11,13 @@ Standard C library
> >  .RI ( libc ", " \-lc )
> >  .SH SYNOPSIS
> >  .nf
> > -.B #include <linux/close_range.h>
> > +.BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
> > +.B #include <unistd.h>
> > +.P
> > +.BR "#include <linux/close_range.h>" "  /* For the flags constants */"
> 
> Could you please format the comment like other pages that do the same
> thing?  See for example membarrier(2).

Sorry, you have to be more explicit what exactly you believe is not
the same thing that other pages do. membarrier.2 doesn't need a
_GNU_SOURCE define and I believe I used the comment as other pages
that do.

Cheers,

Mark

Re: [PATCH v2] close_range.2: Add _GNU_SOURCE and unistd.h to SYNOPSIS

[Alejandro Colomar]

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

Hi Mark,

On Mon, Feb 12, 2024 at 01:05:31AM +0100, Mark Wielaard wrote:
> > >  .SH SYNOPSIS
> > >  .nf
> > > -.B #include <linux/close_range.h>
> > > +.BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
> > > +.B #include <unistd.h>
> > > +.P
> > > +.BR "#include <linux/close_range.h>" "  /* For the flags constants */"
> > 
> > Could you please format the comment like other pages that do the same
> > thing?  See for example membarrier(2).
> 
> Sorry, you have to be more explicit what exactly you believe is not
> the same thing that other pages do. membarrier.2 doesn't need a
> _GNU_SOURCE define and I believe I used the comment as other pages
> that do.

Sure; no problem!

When we include a header not for a function but for constants, we
specify which are those constants.  For example, membarrier(2) has

     #include <linux/membarrier.h> /* Definition of MEMBARRIER_* constants */
     #include <sys/syscall.h>      /* Definition of SYS_* constants */

Here, I'd use
     #include <linux/close_range.h> /* Definition of CLOSE_RANGE_* constants */

BTW, I notice that it's near the 80-column limit.  We try to limit
ourselves to 79 columns, for an obscure reason.  For that, we'll need to
use only 1 space before the comment (otherwise, I'd agree to use 2).

Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

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

Re: [PATCH v2] close_range.2: Add _GNU_SOURCE and unistd.h to SYNOPSIS

[Mark Wielaard]

Hi Alejandro,

On Mon, Feb 12, 2024 at 02:49:16AM +0100, Alejandro Colomar wrote:
> When we include a header not for a function but for constants, we
> specify which are those constants.  For example, membarrier(2) has
> 
>      #include <linux/membarrier.h> /* Definition of MEMBARRIER_* constants */
>      #include <sys/syscall.h>      /* Definition of SYS_* constants */
> 

Aha, ok, the other comment. Will reword for V3.

> Here, I'd use
>      #include <linux/close_range.h> /* Definition of CLOSE_RANGE_* constants */
> 
> BTW, I notice that it's near the 80-column limit.  We try to limit
> ourselves to 79 columns, for an obscure reason.  For that, we'll need to
> use only 1 space before the comment (otherwise, I'd agree to use 2).

Yeah, max 79 max sense (all my terminals are 80 chars wide).
Formatting the comment is a little tricky, but I believe I got it.

Cheers,

Mark