[musl] regression in man pages for interfaces using loff_t

[musl] regression in man pages for interfaces using loff_t

[Rich Felker]

https://github.com/mkerrisk/man-pages/commit/9bebb17e5b5794e495ba8e6c0a75266c65b9d2d7
https://github.com/mkerrisk/man-pages/commit/76c5631fb442f1c7c0b5ec8c653e84f2997249c8

and perhaps related changes for other functions introduced a breaking
change in the documentation for the way applications should call libc
functions for Linux-specific APIs that always take a 64-bit file
offset. The type for this is loff_t, not off64_t, which is an LFS64
type which only exists when LFS64 is supported and enabled.

Applications following the documentation change will not compile on
musl libc, and in theory would not compile even on glibc if it
followed reasonable policy for exposing visibility of off64_t, though
it might happen to be that on glibc, all feature profiles that expose
the relevant functions also expose LFS64.

The whole reason loff_t exists is to avoid this problem and make a
type that's "always full width offset, regardless of _FILE_OFFSET_BITS
or _LARGEFILE64_SOURCE" to match with the kernel expectation for these
interfaces.

For an example of the breakage resulting from following the change in
documentation, see:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110462

Please revert this.

Rich

[musl] Re: regression in man pages for interfaces using loff_t

[Paul Eggert]

On 2023-06-28 10:53, Rich Felker wrote:
> The whole reason loff_t exists is to avoid this problem and make a
> type that's "always full width offset, regardless of _FILE_OFFSET_BITS
> or _LARGEFILE64_SOURCE" to match with the kernel expectation for these
> interfaces.

Why can't off64_t be that type, as it is in glibc? I'm not seeing why we 
need two names for the same type.

[musl] Re: regression in man pages for interfaces using loff_t

[Rich Felker]

On Wed, Jun 28, 2023 at 11:21:39AM -0700, Paul Eggert wrote:
> On 2023-06-28 10:53, Rich Felker wrote:
> >The whole reason loff_t exists is to avoid this problem and make a
> >type that's "always full width offset, regardless of _FILE_OFFSET_BITS
> >or _LARGEFILE64_SOURCE" to match with the kernel expectation for these
> >interfaces.
> 
> Why can't off64_t be that type, as it is in glibc? I'm not seeing
> why we need two names for the same type.

As for why off64_t is not an appropriate type, it's defined by and
associated with the LFS64 summit and the related intefaces, and
governed by them. Using it makes these interfaces non-standardizable,
because no standard is going to adopt a function whose public
interface depends on another optional thing they don't want to
mandate. This is exactly the problem why ISO C is stuck with the
broken and unusable fseek/ftell that take long, and hasn't adopted
fseeko/ftello from POSIX -- their public interfaces use the
POSIX-governed type off_t, and as such, ISO C adopting them without
adopting the whole POSIX off_t is out of the question.

As a particular practical concern, applications performing
configure-like tests may use the existence of an off64_t type to
conclude that the LFS64 API is supported on the system they're being
built on. This is not the case on musl (technically, it's still
supported at build-time with explicit -D_LARGEFILE64_SOURCE, but that
is slated for removal once known issues are fixed) and we don't want
to mislead applications that it is.

There's also the problem that off64_t is "exactly 64-bit" which makes
it unsuitable as an interface type for cross-platform functions where
one could imagine the native type being larger (rather horrifying but
possible). loff_t encodes no such assumption in its name or
specification; it's just "long offset" whatever that is, and an
implementation is free (as we do on musl) to just define it as off_t.

But more fundamentally, the problem here is that a documented
interface was just arbitrarily changed in an incompatible way, with no
process or accountability. The documented interface is what
applications are supposed to be writing to, and what alternate
implementations aiming to be compatible with applications using the
interfaces are supposed to match. Changing that out from under us
later is Not Nice.

Rich

[musl] Re: regression in man pages for interfaces using loff_t

[Paul Eggert]

On 2023-06-28 12:15, Rich Felker wrote:

> There's also the problem that off64_t is "exactly 64-bit" which makes
> it unsuitable as an interface type for cross-platform functions where
> one could imagine the native type being larger (rather horrifying but
> possible).

Although we won't have files with 2**63 bytes any time soon, this is the 
best argument for preferring "loff_t" to "off64_t".

But come to think of it, it'd be better to document the type simply as 
"off_t", with a footnote saying the equivalent of "this assumes that on 
32-bit glibc platforms you compile with -DFILE_OFFSET_BITS=64 like any 
sane person would." The intent really is off_t here, and that will 
remain true even if off_t ever widens past 64 bits.

All the apps I know that use the syscalls in question simply pass 
values that fit in off_t to these functions, and this will work 
regardless of whether these apps are compiled with 64- or (horrors!) 
32-bit off_t. Admittedly the footnote solution would not be perfect, but 
it's good enough, and it would sidestep the loff_t vs off64_t confusion.


> As for why off64_t is not an appropriate type, it's defined by and
> associated with the LFS64 summit and the related intefaces, and
> governed by them. Using it makes these interfaces non-standardizable,
> because no standard is going to adopt a function whose public
> interface depends on another optional thing they don't want to
> mandate.

I don't see why not. For example, POSIX-2018 requires int32_t even 
though C17 says it's optional. So there's precedent for POSIX adopting a 
type that's optional elsewhere.

Also, to POSIX loff_t is just as optional as off64_t is. glibc defines 
neither type if the app #defines _POSIX_C_SOURCE as POSIX requires. So 
from a standardization viewpoint there's no reason to prefer one type 
over the other.


> This is exactly the problem why ISO C is stuck with the
> broken and unusable fseek/ftell that take long, and hasn't adopted
> fseeko/ftello from POSIX -- their public interfaces use the
> POSIX-governed type off_t, and as such, ISO C adopting them without
> adopting the whole POSIX off_t is out of the question.

I'm not sure what the point is here, as far as standardization goes. 
Neither ISO C nor POSIX use loff_t, and neither is likely to ever use 
it: ISO C won't even adopt off_t much less loff_t, and POSIX works just 
fine with off_t and doesn't need loff_t. (The same goes for off64_t of 
course.)


> As a particular practical concern, applications performing
> configure-like tests may use the existence of an off64_t type to
> conclude that the LFS64 API is supported on the system they're being
> built on.

Which apps do that?


But anyway this is all moot if we simply document the arguments as off_t 
with a footnote.

[musl] Re: [libc-coord] Re: regression in man pages for interfaces using loff_t

[Jonathan Wakely]

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

On Fri, 30 Jun 2023 at 08:11, Paul Eggert wrote:

> On 2023-06-28 12:15, Rich Felker wrote:
>
> > There's also the problem that off64_t is "exactly 64-bit" which makes
> > it unsuitable as an interface type for cross-platform functions where
> > one could imagine the native type being larger (rather horrifying but
> > possible).
>
> Although we won't have files with 2**63 bytes any time soon, this is the
> best argument for preferring "loff_t" to "off64_t".
>
> But come to think of it, it'd be better to document the type simply as
> "off_t", with a footnote saying the equivalent of "this assumes that on
> 32-bit glibc platforms you compile with -DFILE_OFFSET_BITS=64 like any
> sane person would." The intent really is off_t here, and that will
> remain true even if off_t ever widens past 64 bits.
>
> All the apps I know that use the syscalls in question simply pass
> values that fit in off_t to these functions, and this will work
> regardless of whether these apps are compiled with 64- or (horrors!)
> 32-bit off_t. Admittedly the footnote solution would not be perfect, but
> it's good enough, and it would sidestep the loff_t vs off64_t confusion.
>

For APIs like copy_file_range(2) and splice(2) the arguments are loff_t* so
you can't just "pass arguments that fit in off_t" to them. You have to get
the pointer type correct, because writing 64-bits through a 32-bit off_t
would be bad. And in C++ it won't even compile unless you get the pointer
types exactly right (C compilers will typically allow the mismatch with
just a warning).

People miss footnotes. I would really prefer if the signature shown in the
man page used a type that will actually compile. If it shows off_t, that
won't compile for 32-bit systems without LFS support enabled.

[-- Attachment #2: Type: text/html, Size: 2220 bytes --]

[musl] Re: [libc-coord] Re: regression in man pages for interfaces using loff_t

[Jonathan Wakely]

On Fri, 30 Jun 2023 at 09:02, Jonathan Wakely wrote:
>
> On Fri, 30 Jun 2023 at 08:11, Paul Eggert wrote:
>>
>> On 2023-06-28 12:15, Rich Felker wrote:
>>
>> > There's also the problem that off64_t is "exactly 64-bit" which makes
>> > it unsuitable as an interface type for cross-platform functions where
>> > one could imagine the native type being larger (rather horrifying but
>> > possible).
>>
>> Although we won't have files with 2**63 bytes any time soon, this is the
>> best argument for preferring "loff_t" to "off64_t".
>>
>> But come to think of it, it'd be better to document the type simply as
>> "off_t", with a footnote saying the equivalent of "this assumes that on
>> 32-bit glibc platforms you compile with -DFILE_OFFSET_BITS=64 like any
>> sane person would." The intent really is off_t here, and that will
>> remain true even if off_t ever widens past 64 bits.
>>
>> All the apps I know that use the syscalls in question simply pass
>> values that fit in off_t to these functions, and this will work
>> regardless of whether these apps are compiled with 64- or (horrors!)
>> 32-bit off_t. Admittedly the footnote solution would not be perfect, but
>> it's good enough, and it would sidestep the loff_t vs off64_t confusion.
>
>
> For APIs like copy_file_range(2) and splice(2) the arguments are loff_t* so you can't just "pass arguments that fit in off_t" to them. You have to get the pointer type correct, because writing 64-bits through a 32-bit off_t would be bad. And in C++ it won't even compile unless you get the pointer types exactly right (C compilers will typically allow the mismatch with just a warning).
>
> People miss footnotes. I would really prefer if the signature shown in the man page used a type that will actually compile. If it shows off_t, that won't compile for 32-bit systems without LFS support enabled.

Apologies for sending the mail above as HTML - replying as text/plain
for those it didn't reach.

[musl] Re: [libc-coord] Re: regression in man pages for interfaces using loff_t

[Sam James]

Jonathan Wakely via Libc-alpha <libc-alpha@sourceware.org> writes:

> On Fri, 30 Jun 2023 at 09:02, Jonathan Wakely wrote:
>>
>> On Fri, 30 Jun 2023 at 08:11, Paul Eggert wrote:
>>>
>>> On 2023-06-28 12:15, Rich Felker wrote:
>>>
>>> > There's also the problem that off64_t is "exactly 64-bit" which makes
>>> > it unsuitable as an interface type for cross-platform functions where
>>> > one could imagine the native type being larger (rather horrifying but
>>> > possible).
>>>
>>> Although we won't have files with 2**63 bytes any time soon, this is the
>>> best argument for preferring "loff_t" to "off64_t".
>>>
>>> But come to think of it, it'd be better to document the type simply as
>>> "off_t", with a footnote saying the equivalent of "this assumes that on
>>> 32-bit glibc platforms you compile with -DFILE_OFFSET_BITS=64 like any
>>> sane person would." The intent really is off_t here, and that will
>>> remain true even if off_t ever widens past 64 bits.
>>>
>>> All the apps I know that use the syscalls in question simply pass
>>> values that fit in off_t to these functions, and this will work
>>> regardless of whether these apps are compiled with 64- or (horrors!)
>>> 32-bit off_t. Admittedly the footnote solution would not be perfect, but
>>> it's good enough, and it would sidestep the loff_t vs off64_t confusion.
>>
>>
>> For APIs like copy_file_range(2) and splice(2) the arguments are
>> loff_t* so you can't just "pass arguments that fit in off_t" to
>> them. You have to get the pointer type correct, because writing
>> 64-bits through a 32-bit off_t would be bad. And in C++ it won't
>> even compile unless you get the pointer types exactly right (C
>> compilers will typically allow the mismatch with just a warning).
>>
>> People miss footnotes. I would really prefer if the signature shown
>> in the man page used a type that will actually compile. If it shows
>> off_t, that won't compile for 32-bit systems without LFS support
>> enabled.
>
> Apologies for sending the mail above as HTML - replying as text/plain
> for those it didn't reach.

Yes, please. The footnote is likely to be misunderstood at best (if
people even see it, they'll think it's some obscure portability issue
and try to #ifdef or something).

Apparently we got confused by this before as well: https://github.com/gentoo/portage/commit/1a7aeb58c4e687e4d4012054b56373f27941a067
(https://bugs.gentoo.org/617778), so I'll need to look at that now too.

[musl] Re: [libc-coord] Re: regression in man pages for interfaces using loff_t

[Paul Eggert]

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

On 2023-06-30 01:02, Jonathan Wakely wrote:

> For APIs like copy_file_range(2) and splice(2) the arguments are loff_t* so
> you can't just "pass arguments that fit in off_t" to them.

Sorry, I missed that detail. Still, the argument stands. On legacy 
32-bit platforms without -D_FILE_OFFSET_BITS=64, calls will still work 
if they pass null pointers to copy_file_range, a common case in my 
experience. The calls that don't, will get typecheck errors or warnings, 
and that's good enough to address the issue.


> And in C++ it won't even compile unless you get the pointer
> types exactly right (C compilers will typically allow the mismatch with
> just a warning).

That's good! People should be using -D_FILE_OFFSET_BITS=64 if they use 
these functions, and the typecheck errors and/or warnings will remind 
them. The man pages don't need to (and shouldn't) document what happens 
if you call these functions on legacy 32-bit platforms without first 
defining _FILE_OFFSET_BITS to be 64.


> People miss footnotes.

OK, let's make the point more prominently, at the start of the man page. 
Proposed patch attached. This patch should work for musl as well as for 
glibc.

[-- Attachment #2: 0001-off64_t-prefer-off_t-for-splice-etc.patch --]
[-- Type: text/x-patch, Size: 8355 bytes --]

From 38bfd1ecda2014955c701f7658a4ab55fa5c8b9d Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 30 Jun 2023 12:25:53 -0700
Subject: [PATCH] off64_t: prefer off_t for splice, etc.

For the few functions that come only in 64-bit off_t flavors,
document their APIs as using off_t instead of off64_t,
and say also that code should #define _FILE_OFFSET_BITS 64.
This documents what user code is (and should be) doing anyway,
if it needs to work on legacy 32-bit Linux.
---
 man2/copy_file_range.2     | 17 ++++++++++++++---
 man2/readahead.2           |  8 +++++++-
 man2/splice.2              | 14 ++++++++++++--
 man2/sync_file_range.2     |  9 +++++++--
 man3/fopencookie.3         | 14 +++++++++++---
 man7/feature_test_macros.7 | 12 ++++++++----
 6 files changed, 59 insertions(+), 15 deletions(-)

diff --git a/man2/copy_file_range.2 b/man2/copy_file_range.2
index 6f3aa4971..bb5aa2223 100644
--- a/man2/copy_file_range.2
+++ b/man2/copy_file_range.2
@@ -11,10 +11,11 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .B #define _GNU_SOURCE
+.B #define _FILE_OFFSET_BITS 64
 .B #include <unistd.h>
 .PP
-.BI "ssize_t copy_file_range(int " fd_in ", off64_t *_Nullable " off_in ,
-.BI "                        int " fd_out ", off64_t *_Nullable " off_out ,
+.BI "ssize_t copy_file_range(int " fd_in ", off_t *_Nullable " off_in ,
+.BI "                        int " fd_out ", off_t *_Nullable " off_out ,
 .BI "                        size_t " len ", unsigned int " flags );
 .fi
 .SH DESCRIPTION
@@ -224,6 +225,15 @@ gives filesystems an opportunity to implement "copy acceleration" techniques,
 such as the use of reflinks (i.e., two or more inodes that share
 pointers to the same copy-on-write disk blocks)
 or server-side-copy (in the case of NFS).
+.PP
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses non-null
+.I off_in
+or
+.I off_out
+or that takes the address of
+.BR copy_file_range ,
+if the code is intended to be portable to legacy 32-bit platforms.
 .SH BUGS
 In Linux 5.3 to Linux 5.18,
 cross-filesystem copies were implemented by the kernel,
@@ -234,6 +244,7 @@ the call failed to copy, while still reporting success.
 .\" SRC BEGIN (copy_file_range.c)
 .EX
 #define _GNU_SOURCE
+#define _FILE_OFFSET_BITS 64
 #include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -244,7 +255,7 @@ int
 main(int argc, char *argv[])
 {
     int          fd_in, fd_out;
-    off64_t      len, ret;
+    off_t        len, ret;
     struct stat  stat;
 \&
     if (argc != 3) {
diff --git a/man2/readahead.2 b/man2/readahead.2
index d69795979..62b9e6786 100644
--- a/man2/readahead.2
+++ b/man2/readahead.2
@@ -14,9 +14,10 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "             /* See feature_test_macros(7) */"
+.B #define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "ssize_t readahead(int " fd ", off64_t " offset ", size_t " count );
+.BI "ssize_t readahead(int " fd ", off_t " offset ", size_t " count );
 .fi
 .SH DESCRIPTION
 .BR readahead ()
@@ -73,6 +74,11 @@ Linux.
 .SH HISTORY
 Linux 2.4.13,
 glibc 2.3.
+.SH NOTES
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses a pointer to
+.BR readahead ,
+if the code is intended to be portable to legacy 32-bit platforms.
 .SH BUGS
 .BR readahead ()
 attempts to schedule the reads in the background and return immediately.
diff --git a/man2/splice.2 b/man2/splice.2
index dd78e8cd4..829d2e336 100644
--- a/man2/splice.2
+++ b/man2/splice.2
@@ -12,10 +12,11 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
+.B "#define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "ssize_t splice(int " fd_in ", off64_t *_Nullable " off_in ,
-.BI "               int " fd_out ", off64_t *_Nullable " off_out ,
+.BI "ssize_t splice(int " fd_in ", off_t *_Nullable " off_in ,
+.BI "               int " fd_out ", off_t *_Nullable " off_out ,
 .BI "               size_t " len ", unsigned int " flags );
 .\" Return type was long before glibc 2.7
 .fi
@@ -242,6 +243,15 @@ only pointers are copied, not the pages of the buffer.
 .\" the data and choose to forward it to two or more different
 .\" users - for things like logging etc.).
 .\"
+.PP
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses non-null
+.I off_in
+or
+.I off_out
+or that takes the address of
+.BR splice ,
+if the code is intended to be portable to legacy 32-bit platforms.
 .SH EXAMPLES
 See
 .BR tee (2).
diff --git a/man2/sync_file_range.2 b/man2/sync_file_range.2
index d633b08ff..0bf17f824 100644
--- a/man2/sync_file_range.2
+++ b/man2/sync_file_range.2
@@ -16,9 +16,10 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
+.B #define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "int sync_file_range(int " fd ", off64_t " offset ", off64_t " nbytes ,
+.BI "int sync_file_range(int " fd ", off_t " offset ", off_t " nbytes ,
 .BI "                    unsigned int " flags );
 .fi
 .SH DESCRIPTION
@@ -176,7 +177,7 @@ system call that orders the arguments suitably:
 .in +4n
 .EX
 .BI "int sync_file_range2(int " fd ", unsigned int " flags ,
-.BI "                     off64_t " offset ", off64_t " nbytes );
+.BI "                     off_t " offset ", off_t " nbytes );
 .EE
 .in
 .PP
@@ -198,6 +199,10 @@ glibc transparently wraps
 under the name
 .BR sync_file_range ().
 .SH NOTES
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that takes the address of
+.BR sync_file_range ,
+if the code is intended to be portable to legacy 32-bit platforms.
 .SH SEE ALSO
 .BR fdatasync (2),
 .BR fsync (2),
diff --git a/man3/fopencookie.3 b/man3/fopencookie.3
index 409a3c81a..08b190394 100644
--- a/man3/fopencookie.3
+++ b/man3/fopencookie.3
@@ -13,6 +13,7 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
+.B #define _FILE_OFFSET_BITS 64
 .B #include <stdio.h>
 .PP
 .BI "FILE *fopencookie(void *restrict " cookie ", const char *restrict " mode ,
@@ -169,7 +170,7 @@ When called, it receives three arguments:
 .IP
 .in +4n
 .EX
-int seek(void *cookie, off64_t *offset, int whence);
+int seek(void *cookie, off_t *offset, int whence);
 .EE
 .in
 .IP
@@ -351,9 +352,9 @@ memfile_read(void *c, char *buf, size_t size)
 }
 \&
 int
-memfile_seek(void *c, off64_t *offset, int whence)
+memfile_seek(void *c, off_t *offset, int whence)
 {
-    off64_t new_offset;
+    off_t new_offset;
     struct memfile_cookie *cookie = c;
 \&
     if (whence == SEEK_SET)
@@ -451,6 +452,13 @@ main(int argc, char *argv[])
 }
 .EE
 .\" SRC END
+.SH NOTES
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses non-null
+.I seek
+or that takes the address of
+.BR fopencookie ,
+if the code is intended to be portable to legacy 32-bit platforms.
 .SH SEE ALSO
 .BR fclose (3),
 .BR fmemopen (3),
diff --git a/man7/feature_test_macros.7 b/man7/feature_test_macros.7
index f1620611c..462fd4abb 100644
--- a/man7/feature_test_macros.7
+++ b/man7/feature_test_macros.7
@@ -113,15 +113,16 @@ feature test macro requirements (this example from
 .RS +4
 .EX
 .B #define _GNU_SOURCE
+.B #define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "ssize_t readahead(int " fd ", off64_t *" offset ", size_t " count );
+.BI "ssize_t readahead(int " fd ", off_t *" offset ", size_t " count );
 .EE
 .RE
 .PP
-This format is employed in cases where only a single
-feature test macro can be used to expose the function
-declaration, and that macro is not defined by default.
+This format is employed in cases where feature macros
+expose the function declaration with the correct type,
+and these macros are not defined by default.
 .SS Feature test macros understood by glibc
 The paragraphs below explain how feature test macros are handled
 in glibc 2.\fIx\fP,
@@ -406,6 +407,9 @@ related to file I/O and filesystem operations into references to
 their 64-bit counterparts.
 This is useful for performing I/O on large files (> 2 Gigabytes)
 on 32-bit systems.
+It is also useful when calling functions like
+.BR copy_file_range (2)
+that were added more recently and that come only in 64-bit flavors.
 (Defining this macro permits correctly written programs to use
 large files with only a recompilation being required.)
 .IP
-- 
2.41.0

Re: [musl] Re: [libc-coord] Re: regression in man pages for interfaces using loff_t

[A. Wilcox]

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

On Jun 30, 2023, at 2:46 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> 
> On 2023-06-30 01:02, Jonathan Wakely wrote:
> 
>> For APIs like copy_file_range(2) and splice(2) the arguments are loff_t* so
>> you can't just "pass arguments that fit in off_t" to them.
> 
> Sorry, I missed that detail. Still, the argument stands. On legacy 32-bit platforms without -D_FILE_OFFSET_BITS=64, calls will still work if they pass null pointers to copy_file_range, a common case in my experience. The calls that don't, will get typecheck errors or warnings, and that's good enough to address the issue.
> 
> 
>> And in C++ it won't even compile unless you get the pointer
>> types exactly right (C compilers will typically allow the mismatch with
>> just a warning).
> 
> That's good! People should be using -D_FILE_OFFSET_BITS=64 if they use these functions, and the typecheck errors and/or warnings will remind them. The man pages don't need to (and shouldn't) document what happens if you call these functions on legacy 32-bit platforms without first defining _FILE_OFFSET_BITS to be 64.
> 
> 
>> People miss footnotes.
> 
> OK, let's make the point more prominently, at the start of the man page. Proposed patch attached. This patch should work for musl as well as for glibc.


The wording around "legacy" 32-bit is very concerning on my end.

_FILE_OFFSET_BITS=32 is legacy, but Linux on 32-bit systems is not.  In fact, that is why we went through all the pain of time64 in Adélie (with our musl friends) - to ensure that 32-bit hardware is fully supported and current.

New designs are still being made with 32-bit Arm chips; there are new ULP x86 32-bit SoCs, etc.

I think anyone casually browsing this documentation could get the wrong idea that 32-bit platforms "in general" are legacy and this should be clarified.

Best,
-A.

--
A. Wilcox (Sent from my iPhone)
Mac, iOS, Linux software engineer

[-- Attachment #2.1: Type: text/html, Size: 1988 bytes --]

[-- Attachment #2.2: 0001-off64_t-prefer-off_t-for-splice-etc.patch --]
[-- Type: application/octet-stream, Size: 8608 bytes --]

From 38bfd1ecda2014955c701f7658a4ab55fa5c8b9d Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 30 Jun 2023 12:25:53 -0700
Subject: [PATCH] off64_t: prefer off_t for splice, etc.

For the few functions that come only in 64-bit off_t flavors,
document their APIs as using off_t instead of off64_t,
and say also that code should #define _FILE_OFFSET_BITS 64.
This documents what user code is (and should be) doing anyway,
if it needs to work on legacy 32-bit Linux.
---
 man2/copy_file_range.2     | 17 ++++++++++++++---
 man2/readahead.2           |  8 +++++++-
 man2/splice.2              | 14 ++++++++++++--
 man2/sync_file_range.2     |  9 +++++++--
 man3/fopencookie.3         | 14 +++++++++++---
 man7/feature_test_macros.7 | 12 ++++++++----
 6 files changed, 59 insertions(+), 15 deletions(-)

diff --git a/man2/copy_file_range.2 b/man2/copy_file_range.2
index 6f3aa4971..bb5aa2223 100644
--- a/man2/copy_file_range.2
+++ b/man2/copy_file_range.2
@@ -11,10 +11,11 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .B #define _GNU_SOURCE
+.B #define _FILE_OFFSET_BITS 64
 .B #include <unistd.h>
 .PP
-.BI "ssize_t copy_file_range(int " fd_in ", off64_t *_Nullable " off_in ,
-.BI "                        int " fd_out ", off64_t *_Nullable " off_out ,
+.BI "ssize_t copy_file_range(int " fd_in ", off_t *_Nullable " off_in ,
+.BI "                        int " fd_out ", off_t *_Nullable " off_out ,
 .BI "                        size_t " len ", unsigned int " flags );
 .fi
 .SH DESCRIPTION
@@ -224,6 +225,15 @@ gives filesystems an opportunity to implement "copy acceleration" techniques,
 such as the use of reflinks (i.e., two or more inodes that share
 pointers to the same copy-on-write disk blocks)
 or server-side-copy (in the case of NFS).
+.PP
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses non-null
+.I off_in
+or
+.I off_out
+or that takes the address of
+.BR copy_file_range ,
+if the code is intended to be portable to legacy 32-bit platforms.
 .SH BUGS
 In Linux 5.3 to Linux 5.18,
 cross-filesystem copies were implemented by the kernel,
@@ -234,6 +244,7 @@ the call failed to copy, while still reporting success.
 .\" SRC BEGIN (copy_file_range.c)
 .EX
 #define _GNU_SOURCE
+#define _FILE_OFFSET_BITS 64
 #include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -244,7 +255,7 @@ int
 main(int argc, char *argv[])
 {
     int          fd_in, fd_out;
-    off64_t      len, ret;
+    off_t        len, ret;
     struct stat  stat;
 \&
     if (argc != 3) {
diff --git a/man2/readahead.2 b/man2/readahead.2
index d69795979..62b9e6786 100644
--- a/man2/readahead.2
+++ b/man2/readahead.2
@@ -14,9 +14,10 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "             /* See feature_test_macros(7) */"
+.B #define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "ssize_t readahead(int " fd ", off64_t " offset ", size_t " count );
+.BI "ssize_t readahead(int " fd ", off_t " offset ", size_t " count );
 .fi
 .SH DESCRIPTION
 .BR readahead ()
@@ -73,6 +74,11 @@ Linux.
 .SH HISTORY
 Linux 2.4.13,
 glibc 2.3.
+.SH NOTES
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses a pointer to
+.BR readahead ,
+if the code is intended to be portable to legacy 32-bit platforms.
 .SH BUGS
 .BR readahead ()
 attempts to schedule the reads in the background and return immediately.
diff --git a/man2/splice.2 b/man2/splice.2
index dd78e8cd4..829d2e336 100644
--- a/man2/splice.2
+++ b/man2/splice.2
@@ -12,10 +12,11 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
+.B "#define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "ssize_t splice(int " fd_in ", off64_t *_Nullable " off_in ,
-.BI "               int " fd_out ", off64_t *_Nullable " off_out ,
+.BI "ssize_t splice(int " fd_in ", off_t *_Nullable " off_in ,
+.BI "               int " fd_out ", off_t *_Nullable " off_out ,
 .BI "               size_t " len ", unsigned int " flags );
 .\" Return type was long before glibc 2.7
 .fi
@@ -242,6 +243,15 @@ only pointers are copied, not the pages of the buffer.
 .\" the data and choose to forward it to two or more different
 .\" users - for things like logging etc.).
 .\"
+.PP
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses non-null
+.I off_in
+or
+.I off_out
+or that takes the address of
+.BR splice ,
+if the code is intended to be portable to legacy 32-bit platforms.
 .SH EXAMPLES
 See
 .BR tee (2).
diff --git a/man2/sync_file_range.2 b/man2/sync_file_range.2
index d633b08ff..0bf17f824 100644
--- a/man2/sync_file_range.2
+++ b/man2/sync_file_range.2
@@ -16,9 +16,10 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
+.B #define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "int sync_file_range(int " fd ", off64_t " offset ", off64_t " nbytes ,
+.BI "int sync_file_range(int " fd ", off_t " offset ", off_t " nbytes ,
 .BI "                    unsigned int " flags );
 .fi
 .SH DESCRIPTION
@@ -176,7 +177,7 @@ system call that orders the arguments suitably:
 .in +4n
 .EX
 .BI "int sync_file_range2(int " fd ", unsigned int " flags ,
-.BI "                     off64_t " offset ", off64_t " nbytes );
+.BI "                     off_t " offset ", off_t " nbytes );
 .EE
 .in
 .PP
@@ -198,6 +199,10 @@ glibc transparently wraps
 under the name
 .BR sync_file_range ().
 .SH NOTES
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that takes the address of
+.BR sync_file_range ,
+if the code is intended to be portable to legacy 32-bit platforms.
 .SH SEE ALSO
 .BR fdatasync (2),
 .BR fsync (2),
diff --git a/man3/fopencookie.3 b/man3/fopencookie.3
index 409a3c81a..08b190394 100644
--- a/man3/fopencookie.3
+++ b/man3/fopencookie.3
@@ -13,6 +13,7 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
+.B #define _FILE_OFFSET_BITS 64
 .B #include <stdio.h>
 .PP
 .BI "FILE *fopencookie(void *restrict " cookie ", const char *restrict " mode ,
@@ -169,7 +170,7 @@ When called, it receives three arguments:
 .IP
 .in +4n
 .EX
-int seek(void *cookie, off64_t *offset, int whence);
+int seek(void *cookie, off_t *offset, int whence);
 .EE
 .in
 .IP
@@ -351,9 +352,9 @@ memfile_read(void *c, char *buf, size_t size)
 }
 \&
 int
-memfile_seek(void *c, off64_t *offset, int whence)
+memfile_seek(void *c, off_t *offset, int whence)
 {
-    off64_t new_offset;
+    off_t new_offset;
     struct memfile_cookie *cookie = c;
 \&
     if (whence == SEEK_SET)
@@ -451,6 +452,13 @@ main(int argc, char *argv[])
 }
 .EE
 .\" SRC END
+.SH NOTES
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses non-null
+.I seek
+or that takes the address of
+.BR fopencookie ,
+if the code is intended to be portable to legacy 32-bit platforms.
 .SH SEE ALSO
 .BR fclose (3),
 .BR fmemopen (3),
diff --git a/man7/feature_test_macros.7 b/man7/feature_test_macros.7
index f1620611c..462fd4abb 100644
--- a/man7/feature_test_macros.7
+++ b/man7/feature_test_macros.7
@@ -113,15 +113,16 @@ feature test macro requirements (this example from
 .RS +4
 .EX
 .B #define _GNU_SOURCE
+.B #define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "ssize_t readahead(int " fd ", off64_t *" offset ", size_t " count );
+.BI "ssize_t readahead(int " fd ", off_t *" offset ", size_t " count );
 .EE
 .RE
 .PP
-This format is employed in cases where only a single
-feature test macro can be used to expose the function
-declaration, and that macro is not defined by default.
+This format is employed in cases where feature macros
+expose the function declaration with the correct type,
+and these macros are not defined by default.
 .SS Feature test macros understood by glibc
 The paragraphs below explain how feature test macros are handled
 in glibc 2.\fIx\fP,
@@ -406,6 +407,9 @@ related to file I/O and filesystem operations into references to
 their 64-bit counterparts.
 This is useful for performing I/O on large files (> 2 Gigabytes)
 on 32-bit systems.
+It is also useful when calling functions like
+.BR copy_file_range (2)
+that were added more recently and that come only in 64-bit flavors.
 (Defining this macro permits correctly written programs to use
 large files with only a recompilation being required.)
 .IP
-- 
2.41.0


[-- Attachment #2.3: Type: text/html, Size: 1108 bytes --]

Re: [musl] Re: [libc-coord] Re: regression in man pages for interfaces using loff_t

[Paul Eggert]

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

On 2023-07-01 18:18, A. Wilcox via Libc-alpha wrote:
> _FILE_OFFSET_BITS=32 is legacy, but Linux on 32-bit systems is not.

Yes, the documentation could be more careful in its wording. As I 
understand it, the only active Linux ports that still support 32-bit 
off_t are x86 and ARM. So in the patch let's change this:

"if the code is intended to be portable to legacy 32-bit platforms."

to this:

"if the code is intended to be portable to traditional 32-bit x86 and 
ARM platforms where off_t's width defaults to 32 bits."

The idea is to let users know the scope of the problem without implying 
that all 32-bit Linux is going away. Revised patch attached.

[-- Attachment #2: 0001-off64_t-prefer-off_t-for-splice-etc.patch --]
[-- Type: text/x-patch, Size: 8707 bytes --]

From ed62eaad1fff242222fca72c537ce8d271f0cd92 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 2 Jul 2023 10:55:53 -0700
Subject: [PATCH v2] off64_t: prefer off_t for splice, etc.

For the few functions that come only in 64-bit off_t flavors,
document their APIs as using off_t instead of off64_t,
and say also that code should #define _FILE_OFFSET_BITS 64.
This documents what user code is (and should be) doing anyway,
if it needs to work on traditional x86 and ARM Linux.
---
 man2/copy_file_range.2     | 20 +++++++++++++++++---
 man2/readahead.2           | 11 ++++++++++-
 man2/splice.2              | 17 +++++++++++++++--
 man2/sync_file_range.2     | 12 ++++++++++--
 man3/fopencookie.3         | 17 ++++++++++++++---
 man7/feature_test_macros.7 | 12 ++++++++----
 6 files changed, 74 insertions(+), 15 deletions(-)

diff --git a/man2/copy_file_range.2 b/man2/copy_file_range.2
index 6f3aa4971..42b950d66 100644
--- a/man2/copy_file_range.2
+++ b/man2/copy_file_range.2
@@ -11,10 +11,11 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .B #define _GNU_SOURCE
+.B #define _FILE_OFFSET_BITS 64
 .B #include <unistd.h>
 .PP
-.BI "ssize_t copy_file_range(int " fd_in ", off64_t *_Nullable " off_in ,
-.BI "                        int " fd_out ", off64_t *_Nullable " off_out ,
+.BI "ssize_t copy_file_range(int " fd_in ", off_t *_Nullable " off_in ,
+.BI "                        int " fd_out ", off_t *_Nullable " off_out ,
 .BI "                        size_t " len ", unsigned int " flags );
 .fi
 .SH DESCRIPTION
@@ -224,6 +225,18 @@ gives filesystems an opportunity to implement "copy acceleration" techniques,
 such as the use of reflinks (i.e., two or more inodes that share
 pointers to the same copy-on-write disk blocks)
 or server-side-copy (in the case of NFS).
+.PP
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses non-null
+.I off_in
+or
+.I off_out
+or that takes the address of
+.BR copy_file_range ,
+if the code is intended to be portable
+to traditional 32-bit x86 and ARM platforms where
+.BR off_t 's
+width defaults to 32 bits.
 .SH BUGS
 In Linux 5.3 to Linux 5.18,
 cross-filesystem copies were implemented by the kernel,
@@ -234,6 +247,7 @@ the call failed to copy, while still reporting success.
 .\" SRC BEGIN (copy_file_range.c)
 .EX
 #define _GNU_SOURCE
+#define _FILE_OFFSET_BITS 64
 #include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -244,7 +258,7 @@ int
 main(int argc, char *argv[])
 {
     int          fd_in, fd_out;
-    off64_t      len, ret;
+    off_t        len, ret;
     struct stat  stat;
 \&
     if (argc != 3) {
diff --git a/man2/readahead.2 b/man2/readahead.2
index d69795979..64e57cdca 100644
--- a/man2/readahead.2
+++ b/man2/readahead.2
@@ -14,9 +14,10 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "             /* See feature_test_macros(7) */"
+.B #define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "ssize_t readahead(int " fd ", off64_t " offset ", size_t " count );
+.BI "ssize_t readahead(int " fd ", off_t " offset ", size_t " count );
 .fi
 .SH DESCRIPTION
 .BR readahead ()
@@ -73,6 +74,14 @@ Linux.
 .SH HISTORY
 Linux 2.4.13,
 glibc 2.3.
+.SH NOTES
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses a pointer to
+.BR readahead ,
+if the code is intended to be portable
+to traditional 32-bit x86 and ARM platforms where
+.BR off_t 's
+width defaults to 32 bits.
 .SH BUGS
 .BR readahead ()
 attempts to schedule the reads in the background and return immediately.
diff --git a/man2/splice.2 b/man2/splice.2
index dd78e8cd4..cd4ed35cb 100644
--- a/man2/splice.2
+++ b/man2/splice.2
@@ -12,10 +12,11 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
+.B "#define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "ssize_t splice(int " fd_in ", off64_t *_Nullable " off_in ,
-.BI "               int " fd_out ", off64_t *_Nullable " off_out ,
+.BI "ssize_t splice(int " fd_in ", off_t *_Nullable " off_in ,
+.BI "               int " fd_out ", off_t *_Nullable " off_out ,
 .BI "               size_t " len ", unsigned int " flags );
 .\" Return type was long before glibc 2.7
 .fi
@@ -242,6 +243,18 @@ only pointers are copied, not the pages of the buffer.
 .\" the data and choose to forward it to two or more different
 .\" users - for things like logging etc.).
 .\"
+.PP
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses non-null
+.I off_in
+or
+.I off_out
+or that takes the address of
+.BR splice ,
+if the code is intended to be portable
+to traditional 32-bit x86 and ARM platforms where
+.BR off_t 's
+width defaults to 32 bits.
 .SH EXAMPLES
 See
 .BR tee (2).
diff --git a/man2/sync_file_range.2 b/man2/sync_file_range.2
index d633b08ff..31d7e5112 100644
--- a/man2/sync_file_range.2
+++ b/man2/sync_file_range.2
@@ -16,9 +16,10 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
+.B #define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "int sync_file_range(int " fd ", off64_t " offset ", off64_t " nbytes ,
+.BI "int sync_file_range(int " fd ", off_t " offset ", off_t " nbytes ,
 .BI "                    unsigned int " flags );
 .fi
 .SH DESCRIPTION
@@ -176,7 +177,7 @@ system call that orders the arguments suitably:
 .in +4n
 .EX
 .BI "int sync_file_range2(int " fd ", unsigned int " flags ,
-.BI "                     off64_t " offset ", off64_t " nbytes );
+.BI "                     off_t " offset ", off_t " nbytes );
 .EE
 .in
 .PP
@@ -198,6 +199,13 @@ glibc transparently wraps
 under the name
 .BR sync_file_range ().
 .SH NOTES
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that takes the address of
+.BR sync_file_range ,
+if the code is intended to be portable
+to traditional 32-bit x86 and ARM platforms where
+.BR off_t 's
+width defaults to 32 bits.
 .SH SEE ALSO
 .BR fdatasync (2),
 .BR fsync (2),
diff --git a/man3/fopencookie.3 b/man3/fopencookie.3
index 409a3c81a..3a68746cc 100644
--- a/man3/fopencookie.3
+++ b/man3/fopencookie.3
@@ -13,6 +13,7 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
+.B #define _FILE_OFFSET_BITS 64
 .B #include <stdio.h>
 .PP
 .BI "FILE *fopencookie(void *restrict " cookie ", const char *restrict " mode ,
@@ -169,7 +170,7 @@ When called, it receives three arguments:
 .IP
 .in +4n
 .EX
-int seek(void *cookie, off64_t *offset, int whence);
+int seek(void *cookie, off_t *offset, int whence);
 .EE
 .in
 .IP
@@ -351,9 +352,9 @@ memfile_read(void *c, char *buf, size_t size)
 }
 \&
 int
-memfile_seek(void *c, off64_t *offset, int whence)
+memfile_seek(void *c, off_t *offset, int whence)
 {
-    off64_t new_offset;
+    off_t new_offset;
     struct memfile_cookie *cookie = c;
 \&
     if (whence == SEEK_SET)
@@ -451,6 +452,16 @@ main(int argc, char *argv[])
 }
 .EE
 .\" SRC END
+.SH NOTES
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses non-null
+.I seek
+or that takes the address of
+.BR fopencookie ,
+if the code is intended to be portable
+to traditional 32-bit x86 and ARM platforms where
+.BR off_t 's
+width defaults to 32 bits.
 .SH SEE ALSO
 .BR fclose (3),
 .BR fmemopen (3),
diff --git a/man7/feature_test_macros.7 b/man7/feature_test_macros.7
index f1620611c..462fd4abb 100644
--- a/man7/feature_test_macros.7
+++ b/man7/feature_test_macros.7
@@ -113,15 +113,16 @@ feature test macro requirements (this example from
 .RS +4
 .EX
 .B #define _GNU_SOURCE
+.B #define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "ssize_t readahead(int " fd ", off64_t *" offset ", size_t " count );
+.BI "ssize_t readahead(int " fd ", off_t *" offset ", size_t " count );
 .EE
 .RE
 .PP
-This format is employed in cases where only a single
-feature test macro can be used to expose the function
-declaration, and that macro is not defined by default.
+This format is employed in cases where feature macros
+expose the function declaration with the correct type,
+and these macros are not defined by default.
 .SS Feature test macros understood by glibc
 The paragraphs below explain how feature test macros are handled
 in glibc 2.\fIx\fP,
@@ -406,6 +407,9 @@ related to file I/O and filesystem operations into references to
 their 64-bit counterparts.
 This is useful for performing I/O on large files (> 2 Gigabytes)
 on 32-bit systems.
+It is also useful when calling functions like
+.BR copy_file_range (2)
+that were added more recently and that come only in 64-bit flavors.
 (Defining this macro permits correctly written programs to use
 large files with only a recompilation being required.)
 .IP
-- 
2.41.0

Re: [musl] Re: [libc-coord] Re: regression in man pages for interfaces using loff_t

[Jakub Wilk]

>-This format is employed in cases where only a single
>-feature test macro can be used to expose the function
>-declaration, and that macro is not defined by default.
>+This format is employed in cases where feature macros
>+expose the function declaration with the correct type,
>+and these macros are not defined by default.

This isn't right. The shorthand format is sometimes used when there's no 
off(64)_t involved, e.g. in memfd_create(2).

-- 
Jakub Wilk

Re: [musl] Re: [libc-coord] Re: regression in man pages for interfaces using loff_t

[Paul Eggert]

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

On 2023-07-03 11:16, Jakub Wilk wrote:
>> -This format is employed in cases where only a single
>> -feature test macro can be used to expose the function
>> -declaration, and that macro is not defined by default.
>> +This format is employed in cases where feature macros
>> +expose the function declaration with the correct type,
>> +and these macros are not defined by default.
> 
> This isn't right. The shorthand format is sometimes used when there's no 
> off(64)_t involved, e.g. in memfd_create(2).

Fair enough. Let's improve that wording to:

This format is employed when the feature test macros ensure that the 
proper function declarations are visible, and the macros are not defined 
by default.

Revised patch attached. If this wording is still not clear enough, 
please feel free to suggest better wording.

[-- Attachment #2: 0001-off64_t-prefer-off_t-for-splice-etc.patch --]
[-- Type: text/x-patch, Size: 8707 bytes --]

From cc84309bafceb801293448f1681f99daa4943817 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 3 Jul 2023 14:32:06 -0700
Subject: [PATCH v3] off64_t: prefer off_t for splice, etc.

For the few functions that come only in 64-bit off_t flavors,
document their APIs as using off_t instead of off64_t,
and say also that code should #define _FILE_OFFSET_BITS 64.
This documents what user code is (and should be) doing anyway,
if it needs to work on traditional x86 and ARM Linux.
---
 man2/copy_file_range.2     | 20 +++++++++++++++++---
 man2/readahead.2           | 11 ++++++++++-
 man2/splice.2              | 17 +++++++++++++++--
 man2/sync_file_range.2     | 12 ++++++++++--
 man3/fopencookie.3         | 17 ++++++++++++++---
 man7/feature_test_macros.7 | 12 ++++++++----
 6 files changed, 74 insertions(+), 15 deletions(-)

diff --git a/man2/copy_file_range.2 b/man2/copy_file_range.2
index 6f3aa4971..42b950d66 100644
--- a/man2/copy_file_range.2
+++ b/man2/copy_file_range.2
@@ -11,10 +11,11 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .B #define _GNU_SOURCE
+.B #define _FILE_OFFSET_BITS 64
 .B #include <unistd.h>
 .PP
-.BI "ssize_t copy_file_range(int " fd_in ", off64_t *_Nullable " off_in ,
-.BI "                        int " fd_out ", off64_t *_Nullable " off_out ,
+.BI "ssize_t copy_file_range(int " fd_in ", off_t *_Nullable " off_in ,
+.BI "                        int " fd_out ", off_t *_Nullable " off_out ,
 .BI "                        size_t " len ", unsigned int " flags );
 .fi
 .SH DESCRIPTION
@@ -224,6 +225,18 @@ gives filesystems an opportunity to implement "copy acceleration" techniques,
 such as the use of reflinks (i.e., two or more inodes that share
 pointers to the same copy-on-write disk blocks)
 or server-side-copy (in the case of NFS).
+.PP
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses non-null
+.I off_in
+or
+.I off_out
+or that takes the address of
+.BR copy_file_range ,
+if the code is intended to be portable
+to traditional 32-bit x86 and ARM platforms where
+.BR off_t 's
+width defaults to 32 bits.
 .SH BUGS
 In Linux 5.3 to Linux 5.18,
 cross-filesystem copies were implemented by the kernel,
@@ -234,6 +247,7 @@ the call failed to copy, while still reporting success.
 .\" SRC BEGIN (copy_file_range.c)
 .EX
 #define _GNU_SOURCE
+#define _FILE_OFFSET_BITS 64
 #include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -244,7 +258,7 @@ int
 main(int argc, char *argv[])
 {
     int          fd_in, fd_out;
-    off64_t      len, ret;
+    off_t        len, ret;
     struct stat  stat;
 \&
     if (argc != 3) {
diff --git a/man2/readahead.2 b/man2/readahead.2
index d69795979..64e57cdca 100644
--- a/man2/readahead.2
+++ b/man2/readahead.2
@@ -14,9 +14,10 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "             /* See feature_test_macros(7) */"
+.B #define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "ssize_t readahead(int " fd ", off64_t " offset ", size_t " count );
+.BI "ssize_t readahead(int " fd ", off_t " offset ", size_t " count );
 .fi
 .SH DESCRIPTION
 .BR readahead ()
@@ -73,6 +74,14 @@ Linux.
 .SH HISTORY
 Linux 2.4.13,
 glibc 2.3.
+.SH NOTES
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses a pointer to
+.BR readahead ,
+if the code is intended to be portable
+to traditional 32-bit x86 and ARM platforms where
+.BR off_t 's
+width defaults to 32 bits.
 .SH BUGS
 .BR readahead ()
 attempts to schedule the reads in the background and return immediately.
diff --git a/man2/splice.2 b/man2/splice.2
index dd78e8cd4..cd4ed35cb 100644
--- a/man2/splice.2
+++ b/man2/splice.2
@@ -12,10 +12,11 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
+.B "#define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "ssize_t splice(int " fd_in ", off64_t *_Nullable " off_in ,
-.BI "               int " fd_out ", off64_t *_Nullable " off_out ,
+.BI "ssize_t splice(int " fd_in ", off_t *_Nullable " off_in ,
+.BI "               int " fd_out ", off_t *_Nullable " off_out ,
 .BI "               size_t " len ", unsigned int " flags );
 .\" Return type was long before glibc 2.7
 .fi
@@ -242,6 +243,18 @@ only pointers are copied, not the pages of the buffer.
 .\" the data and choose to forward it to two or more different
 .\" users - for things like logging etc.).
 .\"
+.PP
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses non-null
+.I off_in
+or
+.I off_out
+or that takes the address of
+.BR splice ,
+if the code is intended to be portable
+to traditional 32-bit x86 and ARM platforms where
+.BR off_t 's
+width defaults to 32 bits.
 .SH EXAMPLES
 See
 .BR tee (2).
diff --git a/man2/sync_file_range.2 b/man2/sync_file_range.2
index d633b08ff..31d7e5112 100644
--- a/man2/sync_file_range.2
+++ b/man2/sync_file_range.2
@@ -16,9 +16,10 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
+.B #define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "int sync_file_range(int " fd ", off64_t " offset ", off64_t " nbytes ,
+.BI "int sync_file_range(int " fd ", off_t " offset ", off_t " nbytes ,
 .BI "                    unsigned int " flags );
 .fi
 .SH DESCRIPTION
@@ -176,7 +177,7 @@ system call that orders the arguments suitably:
 .in +4n
 .EX
 .BI "int sync_file_range2(int " fd ", unsigned int " flags ,
-.BI "                     off64_t " offset ", off64_t " nbytes );
+.BI "                     off_t " offset ", off_t " nbytes );
 .EE
 .in
 .PP
@@ -198,6 +199,13 @@ glibc transparently wraps
 under the name
 .BR sync_file_range ().
 .SH NOTES
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that takes the address of
+.BR sync_file_range ,
+if the code is intended to be portable
+to traditional 32-bit x86 and ARM platforms where
+.BR off_t 's
+width defaults to 32 bits.
 .SH SEE ALSO
 .BR fdatasync (2),
 .BR fsync (2),
diff --git a/man3/fopencookie.3 b/man3/fopencookie.3
index 409a3c81a..3a68746cc 100644
--- a/man3/fopencookie.3
+++ b/man3/fopencookie.3
@@ -13,6 +13,7 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
+.B #define _FILE_OFFSET_BITS 64
 .B #include <stdio.h>
 .PP
 .BI "FILE *fopencookie(void *restrict " cookie ", const char *restrict " mode ,
@@ -169,7 +170,7 @@ When called, it receives three arguments:
 .IP
 .in +4n
 .EX
-int seek(void *cookie, off64_t *offset, int whence);
+int seek(void *cookie, off_t *offset, int whence);
 .EE
 .in
 .IP
@@ -351,9 +352,9 @@ memfile_read(void *c, char *buf, size_t size)
 }
 \&
 int
-memfile_seek(void *c, off64_t *offset, int whence)
+memfile_seek(void *c, off_t *offset, int whence)
 {
-    off64_t new_offset;
+    off_t new_offset;
     struct memfile_cookie *cookie = c;
 \&
     if (whence == SEEK_SET)
@@ -451,6 +452,16 @@ main(int argc, char *argv[])
 }
 .EE
 .\" SRC END
+.SH NOTES
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses non-null
+.I seek
+or that takes the address of
+.BR fopencookie ,
+if the code is intended to be portable
+to traditional 32-bit x86 and ARM platforms where
+.BR off_t 's
+width defaults to 32 bits.
 .SH SEE ALSO
 .BR fclose (3),
 .BR fmemopen (3),
diff --git a/man7/feature_test_macros.7 b/man7/feature_test_macros.7
index f1620611c..10e973dbc 100644
--- a/man7/feature_test_macros.7
+++ b/man7/feature_test_macros.7
@@ -113,15 +113,16 @@ feature test macro requirements (this example from
 .RS +4
 .EX
 .B #define _GNU_SOURCE
+.B #define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "ssize_t readahead(int " fd ", off64_t *" offset ", size_t " count );
+.BI "ssize_t readahead(int " fd ", off_t *" offset ", size_t " count );
 .EE
 .RE
 .PP
-This format is employed in cases where only a single
-feature test macro can be used to expose the function
-declaration, and that macro is not defined by default.
+This format is employed when the feature test macros ensure
+that the proper function declarations are visible,
+and the macros are not defined by default.
 .SS Feature test macros understood by glibc
 The paragraphs below explain how feature test macros are handled
 in glibc 2.\fIx\fP,
@@ -406,6 +407,9 @@ related to file I/O and filesystem operations into references to
 their 64-bit counterparts.
 This is useful for performing I/O on large files (> 2 Gigabytes)
 on 32-bit systems.
+It is also useful when calling functions like
+.BR copy_file_range (2)
+that were added more recently and that come only in 64-bit flavors.
 (Defining this macro permits correctly written programs to use
 large files with only a recompilation being required.)
 .IP
-- 
2.41.0

Re: [musl] Re: [libc-coord] Re: regression in man pages for interfaces using loff_t

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 1286 bytes --]

Hi all,

On 7/3/23 23:35, Paul Eggert wrote:
> On 2023-07-03 11:16, Jakub Wilk wrote:
>>> -This format is employed in cases where only a single
>>> -feature test macro can be used to expose the function
>>> -declaration, and that macro is not defined by default.
>>> +This format is employed in cases where feature macros
>>> +expose the function declaration with the correct type,
>>> +and these macros are not defined by default.
>>
>> This isn't right. The shorthand format is sometimes used when there's 
>> no off(64)_t involved, e.g. in memfd_create(2).
> 
> Fair enough. Let's improve that wording to:
> 
> This format is employed when the feature test macros ensure that the 
> proper function declarations are visible, and the macros are not defined 
> by default.
> 
> Revised patch attached. If this wording is still not clear enough, 
> please feel free to suggest better wording.

Thanks for the v3 patch, Paul.  I like it.  Would you mind resending it
inline, to make it easier to quote-reply to it, in case anyone wants
to discuss anything?  Does anyone oppose to this patch, and wants to
propose an alternative patch?

Cheers,
Alex

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


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

[musl] [PATCH v4] off64_t: prefer off_t for splice, etc.

[Paul Eggert]

For the few functions that come only in 64-bit off_t flavors,
document their APIs as using off_t instead of off64_t,
and say also that code should #define _FILE_OFFSET_BITS 64.
This documents what user code is (and should be) doing anyway,
if it needs to work on traditional x86 and ARM Linux.
---
 man2/copy_file_range.2     | 20 +++++++++++++++++---
 man2/readahead.2           | 11 ++++++++++-
 man2/splice.2              | 17 +++++++++++++++--
 man2/sync_file_range.2     | 12 ++++++++++--
 man3/fopencookie.3         | 17 ++++++++++++++---
 man7/feature_test_macros.7 | 12 ++++++++----
 6 files changed, 74 insertions(+), 15 deletions(-)

diff --git a/man2/copy_file_range.2 b/man2/copy_file_range.2
index 6f3aa4971..42b950d66 100644
--- a/man2/copy_file_range.2
+++ b/man2/copy_file_range.2
@@ -11,10 +11,11 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .B #define _GNU_SOURCE
+.B #define _FILE_OFFSET_BITS 64
 .B #include <unistd.h>
 .PP
-.BI "ssize_t copy_file_range(int " fd_in ", off64_t *_Nullable " off_in ,
-.BI "                        int " fd_out ", off64_t *_Nullable " off_out ,
+.BI "ssize_t copy_file_range(int " fd_in ", off_t *_Nullable " off_in ,
+.BI "                        int " fd_out ", off_t *_Nullable " off_out ,
 .BI "                        size_t " len ", unsigned int " flags );
 .fi
 .SH DESCRIPTION
@@ -224,6 +225,18 @@ gives filesystems an opportunity to implement "copy acceleration" techniques,
 such as the use of reflinks (i.e., two or more inodes that share
 pointers to the same copy-on-write disk blocks)
 or server-side-copy (in the case of NFS).
+.PP
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses non-null
+.I off_in
+or
+.I off_out
+or that takes the address of
+.BR copy_file_range ,
+if the code is intended to be portable
+to traditional 32-bit x86 and ARM platforms where
+.BR off_t 's
+width defaults to 32 bits.
 .SH BUGS
 In Linux 5.3 to Linux 5.18,
 cross-filesystem copies were implemented by the kernel,
@@ -234,6 +247,7 @@ the call failed to copy, while still reporting success.
 .\" SRC BEGIN (copy_file_range.c)
 .EX
 #define _GNU_SOURCE
+#define _FILE_OFFSET_BITS 64
 #include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -244,7 +258,7 @@ int
 main(int argc, char *argv[])
 {
     int          fd_in, fd_out;
-    off64_t      len, ret;
+    off_t        len, ret;
     struct stat  stat;
 \&
     if (argc != 3) {
diff --git a/man2/readahead.2 b/man2/readahead.2
index d69795979..64e57cdca 100644
--- a/man2/readahead.2
+++ b/man2/readahead.2
@@ -14,9 +14,10 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "             /* See feature_test_macros(7) */"
+.B #define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "ssize_t readahead(int " fd ", off64_t " offset ", size_t " count );
+.BI "ssize_t readahead(int " fd ", off_t " offset ", size_t " count );
 .fi
 .SH DESCRIPTION
 .BR readahead ()
@@ -73,6 +74,14 @@ Linux.
 .SH HISTORY
 Linux 2.4.13,
 glibc 2.3.
+.SH NOTES
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses a pointer to
+.BR readahead ,
+if the code is intended to be portable
+to traditional 32-bit x86 and ARM platforms where
+.BR off_t 's
+width defaults to 32 bits.
 .SH BUGS
 .BR readahead ()
 attempts to schedule the reads in the background and return immediately.
diff --git a/man2/splice.2 b/man2/splice.2
index dd78e8cd4..cd4ed35cb 100644
--- a/man2/splice.2
+++ b/man2/splice.2
@@ -12,10 +12,11 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
+.B "#define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "ssize_t splice(int " fd_in ", off64_t *_Nullable " off_in ,
-.BI "               int " fd_out ", off64_t *_Nullable " off_out ,
+.BI "ssize_t splice(int " fd_in ", off_t *_Nullable " off_in ,
+.BI "               int " fd_out ", off_t *_Nullable " off_out ,
 .BI "               size_t " len ", unsigned int " flags );
 .\" Return type was long before glibc 2.7
 .fi
@@ -242,6 +243,18 @@ only pointers are copied, not the pages of the buffer.
 .\" the data and choose to forward it to two or more different
 .\" users - for things like logging etc.).
 .\"
+.PP
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses non-null
+.I off_in
+or
+.I off_out
+or that takes the address of
+.BR splice ,
+if the code is intended to be portable
+to traditional 32-bit x86 and ARM platforms where
+.BR off_t 's
+width defaults to 32 bits.
 .SH EXAMPLES
 See
 .BR tee (2).
diff --git a/man2/sync_file_range.2 b/man2/sync_file_range.2
index d633b08ff..31d7e5112 100644
--- a/man2/sync_file_range.2
+++ b/man2/sync_file_range.2
@@ -16,9 +16,10 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
+.B #define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "int sync_file_range(int " fd ", off64_t " offset ", off64_t " nbytes ,
+.BI "int sync_file_range(int " fd ", off_t " offset ", off_t " nbytes ,
 .BI "                    unsigned int " flags );
 .fi
 .SH DESCRIPTION
@@ -176,7 +177,7 @@ system call that orders the arguments suitably:
 .in +4n
 .EX
 .BI "int sync_file_range2(int " fd ", unsigned int " flags ,
-.BI "                     off64_t " offset ", off64_t " nbytes );
+.BI "                     off_t " offset ", off_t " nbytes );
 .EE
 .in
 .PP
@@ -198,6 +199,13 @@ glibc transparently wraps
 under the name
 .BR sync_file_range ().
 .SH NOTES
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that takes the address of
+.BR sync_file_range ,
+if the code is intended to be portable
+to traditional 32-bit x86 and ARM platforms where
+.BR off_t 's
+width defaults to 32 bits.
 .SH SEE ALSO
 .BR fdatasync (2),
 .BR fsync (2),
diff --git a/man3/fopencookie.3 b/man3/fopencookie.3
index 409a3c81a..3a68746cc 100644
--- a/man3/fopencookie.3
+++ b/man3/fopencookie.3
@@ -13,6 +13,7 @@ Standard C library
 .SH SYNOPSIS
 .nf
 .BR "#define _GNU_SOURCE" "         /* See feature_test_macros(7) */"
+.B #define _FILE_OFFSET_BITS 64
 .B #include <stdio.h>
 .PP
 .BI "FILE *fopencookie(void *restrict " cookie ", const char *restrict " mode ,
@@ -169,7 +170,7 @@ When called, it receives three arguments:
 .IP
 .in +4n
 .EX
-int seek(void *cookie, off64_t *offset, int whence);
+int seek(void *cookie, off_t *offset, int whence);
 .EE
 .in
 .IP
@@ -351,9 +352,9 @@ memfile_read(void *c, char *buf, size_t size)
 }
 \&
 int
-memfile_seek(void *c, off64_t *offset, int whence)
+memfile_seek(void *c, off_t *offset, int whence)
 {
-    off64_t new_offset;
+    off_t new_offset;
     struct memfile_cookie *cookie = c;
 \&
     if (whence == SEEK_SET)
@@ -451,6 +452,16 @@ main(int argc, char *argv[])
 }
 .EE
 .\" SRC END
+.SH NOTES
+.B _FILE_OFFSET_BITS
+should be defined to be 64 in code that uses non-null
+.I seek
+or that takes the address of
+.BR fopencookie ,
+if the code is intended to be portable
+to traditional 32-bit x86 and ARM platforms where
+.BR off_t 's
+width defaults to 32 bits.
 .SH SEE ALSO
 .BR fclose (3),
 .BR fmemopen (3),
diff --git a/man7/feature_test_macros.7 b/man7/feature_test_macros.7
index f1620611c..10e973dbc 100644
--- a/man7/feature_test_macros.7
+++ b/man7/feature_test_macros.7
@@ -113,15 +113,16 @@ feature test macro requirements (this example from
 .RS +4
 .EX
 .B #define _GNU_SOURCE
+.B #define _FILE_OFFSET_BITS 64
 .B #include <fcntl.h>
 .PP
-.BI "ssize_t readahead(int " fd ", off64_t *" offset ", size_t " count );
+.BI "ssize_t readahead(int " fd ", off_t *" offset ", size_t " count );
 .EE
 .RE
 .PP
-This format is employed in cases where only a single
-feature test macro can be used to expose the function
-declaration, and that macro is not defined by default.
+This format is employed when the feature test macros ensure
+that the proper function declarations are visible,
+and the macros are not defined by default.
 .SS Feature test macros understood by glibc
 The paragraphs below explain how feature test macros are handled
 in glibc 2.\fIx\fP,
@@ -406,6 +407,9 @@ related to file I/O and filesystem operations into references to
 their 64-bit counterparts.
 This is useful for performing I/O on large files (> 2 Gigabytes)
 on 32-bit systems.
+It is also useful when calling functions like
+.BR copy_file_range (2)
+that were added more recently and that come only in 64-bit flavors.
 (Defining this macro permits correctly written programs to use
 large files with only a recompilation being required.)
 .IP
-- 
2.41.0

[musl] Re: [libc-coord] [PATCH v4] off64_t: prefer off_t for splice, etc.

[Sam James]

Paul Eggert <eggert@cs.ucla.edu> writes:

> For the few functions that come only in 64-bit off_t flavors,
> document their APIs as using off_t instead of off64_t,
> and say also that code should #define _FILE_OFFSET_BITS 64.
> This documents what user code is (and should be) doing anyway,
> if it needs to work on traditional x86 and ARM Linux.

LGTM and thank you Paul.

I haven't checked for other prototypes/examples which need
changing.

[musl] Re: [libc-coord] [PATCH v4] off64_t: prefer off_t for splice, etc.

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 2235 bytes --]

Hi Paul, Sam, and Rich,

On 2023-07-09 08:16, Sam James wrote:
> 
> Paul Eggert <eggert@cs.ucla.edu> writes:
> 
>> For the few functions that come only in 64-bit off_t flavors,
>> document their APIs as using off_t instead of off64_t,
>> and say also that code should #define _FILE_OFFSET_BITS 64.
>> This documents what user code is (and should be) doing anyway,
>> if it needs to work on traditional x86 and ARM Linux.
> 
> LGTM and thank you Paul.
> 
> I haven't checked for other prototypes/examples which need
> changing.

Thanks, I'm going to apply the patch.  Can you please confirm if I'm
correct in adding the following tags?

    Reported-by: Rich Felker <dalias@libc.org>
    Fixes: 9bebb17e5b57 ("splice.2: Use 'off64_t' instead of 'loff_t'")
    Fixes: 76c5631fb442 ("copy_file_range.2: Document glibc wrapper instead of kernel syscall")
    Fixes: 5cabfa06b407 ("man-pages 1.68")
    Fixes: 3ca974e3988a ("New page for sync_file_range(2), new in kernel 2.6.17.")
    Fixes: 9bebb17e5b57 ("sync_file_range.2: Document the architecture-specific sync_file_range2() system call")
    Fixes: 79bf8cdcf36a ("Document fopencookie(3), a library function that allows custom implementation of a stdio stream.")
    Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
    Reviewed-by: Sam James <sam@gentoo.org>
    Cc: Jonathan Wakely <jwakely@redhat.com>
    Cc: Szabolcs Nagy <nsz@port70.net>
    Cc: Jakub Wilk <jwilk@jwilk.net>
    Cc: A. Wilcox <AWilcox@wilcox-tech.com>
    Signed-off-by: Alejandro Colomar <alx@kernel.org>


BTW, Rich, please note the commits that this fixes: most of them are
the initial commit that adds a page, which means that the function
had always been documented with off64_t in the "spec".  Only splice(2)
and copy_file_range(2) have been adjusted afterwards, and in a manner
to be consistent with the rest of the pages, so I can only conclude
that we didn't break the spec, but rather fixed it.

Nevertheless, I'm sorry that it caused any problems to musl, and I'm
happy that you reported them and so we can now improve the pages.

Cheers,
Alex

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


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

Re: [musl] Re: [libc-coord] [PATCH v4] off64_t: prefer off_t for splice, etc.

[Rich Felker]

On Sat, Jul 15, 2023 at 05:08:18PM +0200, Alejandro Colomar wrote:
> Hi Paul, Sam, and Rich,
> 
> On 2023-07-09 08:16, Sam James wrote:
> > 
> > Paul Eggert <eggert@cs.ucla.edu> writes:
> > 
> >> For the few functions that come only in 64-bit off_t flavors,
> >> document their APIs as using off_t instead of off64_t,
> >> and say also that code should #define _FILE_OFFSET_BITS 64.
> >> This documents what user code is (and should be) doing anyway,
> >> if it needs to work on traditional x86 and ARM Linux.
> > 
> > LGTM and thank you Paul.
> > 
> > I haven't checked for other prototypes/examples which need
> > changing.
> 
> Thanks, I'm going to apply the patch.  Can you please confirm if I'm
> correct in adding the following tags?
> 
>     Reported-by: Rich Felker <dalias@libc.org>
>     Fixes: 9bebb17e5b57 ("splice.2: Use 'off64_t' instead of 'loff_t'")
>     Fixes: 76c5631fb442 ("copy_file_range.2: Document glibc wrapper instead of kernel syscall")
>     Fixes: 5cabfa06b407 ("man-pages 1.68")
>     Fixes: 3ca974e3988a ("New page for sync_file_range(2), new in kernel 2.6.17.")
>     Fixes: 9bebb17e5b57 ("sync_file_range.2: Document the architecture-specific sync_file_range2() system call")
>     Fixes: 79bf8cdcf36a ("Document fopencookie(3), a library function that allows custom implementation of a stdio stream.")
>     Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
>     Reviewed-by: Sam James <sam@gentoo.org>
>     Cc: Jonathan Wakely <jwakely@redhat.com>
>     Cc: Szabolcs Nagy <nsz@port70.net>
>     Cc: Jakub Wilk <jwilk@jwilk.net>
>     Cc: A. Wilcox <AWilcox@wilcox-tech.com>
>     Signed-off-by: Alejandro Colomar <alx@kernel.org>
> 
> 
> BTW, Rich, please note the commits that this fixes: most of them are
> the initial commit that adds a page, which means that the function
> had always been documented with off64_t in the "spec".  Only splice(2)
> and copy_file_range(2) have been adjusted afterwards, and in a manner
> to be consistent with the rest of the pages, so I can only conclude
> that we didn't break the spec, but rather fixed it.
> 
> Nevertheless, I'm sorry that it caused any problems to musl, and I'm
> happy that you reported them and so we can now improve the pages.

While I like off_t, I am still unhappy that this seems to have been a
unilateral action from documentation side without even hearing input
from any major implementors other than myself. Is "you can't use these
interfaces without -D_FILE_OFFSET_BITS=64" an acceptable outcome to
the glibc folks?

Rich

Re: [musl] Re: [libc-coord] [PATCH v4] off64_t: prefer off_t for splice, etc.

[Paul Eggert]

On 2023-07-15 11:35, Rich Felker wrote:
> Is "you can't use these
> interfaces without -D_FILE_OFFSET_BITS=64" an acceptable outcome to
> the glibc folks?

You can ask on libc-alpha if you like. I'll follow up if necessary.

Re: [musl] Re: [libc-coord] [PATCH v4] off64_t: prefer off_t for splice, etc.

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 5641 bytes --]

[CC += glibc, enh]

Hi Rich,

On 2023-07-15 20:35, Rich Felker wrote:
> On Sat, Jul 15, 2023 at 05:08:18PM +0200, Alejandro Colomar wrote:
>> Hi Paul, Sam, and Rich,
>>
>> On 2023-07-09 08:16, Sam James wrote:
>>>
>>> Paul Eggert <eggert@cs.ucla.edu> writes:
>>>
>>>> For the few functions that come only in 64-bit off_t flavors,
>>>> document their APIs as using off_t instead of off64_t,
>>>> and say also that code should #define _FILE_OFFSET_BITS 64.
>>>> This documents what user code is (and should be) doing anyway,
>>>> if it needs to work on traditional x86 and ARM Linux.
>>>
>>> LGTM and thank you Paul.
>>>
>>> I haven't checked for other prototypes/examples which need
>>> changing.
>>
>> Thanks, I'm going to apply the patch.  Can you please confirm if I'm
>> correct in adding the following tags?
>>
>>     Reported-by: Rich Felker <dalias@libc.org>
>>     Fixes: 9bebb17e5b57 ("splice.2: Use 'off64_t' instead of 'loff_t'")
>>     Fixes: 76c5631fb442 ("copy_file_range.2: Document glibc wrapper instead of kernel syscall")
>>     Fixes: 5cabfa06b407 ("man-pages 1.68")
>>     Fixes: 3ca974e3988a ("New page for sync_file_range(2), new in kernel 2.6.17.")
>>     Fixes: 9bebb17e5b57 ("sync_file_range.2: Document the architecture-specific sync_file_range2() system call")
>>     Fixes: 79bf8cdcf36a ("Document fopencookie(3), a library function that allows custom implementation of a stdio stream.")
>>     Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
>>     Reviewed-by: Sam James <sam@gentoo.org>
>>     Cc: Jonathan Wakely <jwakely@redhat.com>
>>     Cc: Szabolcs Nagy <nsz@port70.net>
>>     Cc: Jakub Wilk <jwilk@jwilk.net>
>>     Cc: A. Wilcox <AWilcox@wilcox-tech.com>
>>     Signed-off-by: Alejandro Colomar <alx@kernel.org>
>>
>>
>> BTW, Rich, please note the commits that this fixes: most of them are
>> the initial commit that adds a page, which means that the function
>> had always been documented with off64_t in the "spec".  Only splice(2)
>> and copy_file_range(2) have been adjusted afterwards, and in a manner
>> to be consistent with the rest of the pages, so I can only conclude
>> that we didn't break the spec, but rather fixed it.
>>
>> Nevertheless, I'm sorry that it caused any problems to musl, and I'm
>> happy that you reported them and so we can now improve the pages.
> 
> While I like off_t, I am still unhappy that this seems to have been a
> unilateral action from documentation side without even hearing input
> from any major implementors other than myself.

Since Paul Eggert is a glibc maintainer, I consider his input as being
representative enough of glibc, even with the list not in CC.  He
usually produces very high quality patches, and I know he has special
care about very odd platforms where type sizes are not usual.

We also had the review and approval of a distribution maintainer, Sam,
so I wouldn't call this unilateral.

It's true we didn't ask the entire glibc list.  I'm fixing that by
adding libc-alpha@ to the loop; let's see if they have anything to say
about the patch, which BTW I pushed a few hours ago.  And hell, while
we're at it, I'm CCing enh from bionic too, just in case he has any
opinion (although he probably read this thread from the linux-man@
list).

For future times, when opening a thread like this where input from
glibc (or kernel) maintainers is not only welcome but essential, it's
usually better to include the relevant list in CC right from the very
first email, to provide them with full context, as I suggest in the
CONTRIBUTING file.

And while at it, I'm thinking that maybe we should mention musl's
list in CONTRIBUTING too.  Should I apply the patch below?  Would you
mind reading that file, and suggesting anything you want for
preventing similar conflicts with musl in the future?

Thank you all!
Alex

diff --git a/CONTRIBUTING b/CONTRIBUTING
index 80052c38e..a030b54a5 100644
--- a/CONTRIBUTING
+++ b/CONTRIBUTING
@@ -27,12 +27,13 @@ Description
        discussed in a man-pages email, please identify yourself as such.
        Relevant mailing lists may include:
 
            Cc: LKML <linux-kernel@vger.kernel.org>
            Cc: Linux API <linux-api@vger.kernel.org>
            Cc: Glibc <libc-alpha@sourceware.org>
+           Cc: musl libc <musl@lists.openwall.com>
 
        For other kernel mailing lists and maintainers, check the
        <MAINTAINERS> file in the Linux kernel repository.
 
        Please don't send HTML email; it will be discarded by the list.
 
@@ -186,13 +187,13 @@ Description
 Reporting bugs
        Report bugs to the mailing list, following the instructions above
        for sending mails to the list.  If you can write a patch (see
        instructions for sending patches above), it's preferred.
 
        If you're unsure if the bug is in the manual page or in the code
-       being documented (kernel, glibc, ...), it's best to send the
+       being documented (kernel, libc, ...), it's best to send the
        report to both at the same time, that is, CC all the mailing
        lists that may be concerned by the report.
 
        Some distributions (for example Debian) apply patches to the
        upstream manual pages.  If you suspect the bug is in one of those
        patches, report it to your distribution maintainer.


> Is "you can't use these
> interfaces without -D_FILE_OFFSET_BITS=64" an acceptable outcome to
> the glibc folks?
> 
> Rich

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


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

Re: [musl] Re: [libc-coord] [PATCH v4] off64_t: prefer off_t for splice, etc.

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 6165 bytes --]

I forgot to refer to the start of the thread:
<https://lore.kernel.org/linux-man/20230628191947.GE3630668@port70.net/T/#t>

The patch applied was this one:
<https://lore.kernel.org/linux-man/20230628191947.GE3630668@port70.net/T/#md5cb63f26575d9ecf5753027e7a87c2782d0d872>

On 2023-07-16 02:35, Alejandro Colomar wrote:
> [CC += glibc, enh]
> 
> Hi Rich,
> 
> On 2023-07-15 20:35, Rich Felker wrote:
>> On Sat, Jul 15, 2023 at 05:08:18PM +0200, Alejandro Colomar wrote:
>>> Hi Paul, Sam, and Rich,
>>>
>>> On 2023-07-09 08:16, Sam James wrote:
>>>>
>>>> Paul Eggert <eggert@cs.ucla.edu> writes:
>>>>
>>>>> For the few functions that come only in 64-bit off_t flavors,
>>>>> document their APIs as using off_t instead of off64_t,
>>>>> and say also that code should #define _FILE_OFFSET_BITS 64.
>>>>> This documents what user code is (and should be) doing anyway,
>>>>> if it needs to work on traditional x86 and ARM Linux.
>>>>
>>>> LGTM and thank you Paul.
>>>>
>>>> I haven't checked for other prototypes/examples which need
>>>> changing.
>>>
>>> Thanks, I'm going to apply the patch.  Can you please confirm if I'm
>>> correct in adding the following tags?
>>>
>>>     Reported-by: Rich Felker <dalias@libc.org>
>>>     Fixes: 9bebb17e5b57 ("splice.2: Use 'off64_t' instead of 'loff_t'")
>>>     Fixes: 76c5631fb442 ("copy_file_range.2: Document glibc wrapper instead of kernel syscall")
>>>     Fixes: 5cabfa06b407 ("man-pages 1.68")
>>>     Fixes: 3ca974e3988a ("New page for sync_file_range(2), new in kernel 2.6.17.")
>>>     Fixes: 9bebb17e5b57 ("sync_file_range.2: Document the architecture-specific sync_file_range2() system call")
>>>     Fixes: 79bf8cdcf36a ("Document fopencookie(3), a library function that allows custom implementation of a stdio stream.")
>>>     Signed-off-by: Paul Eggert <eggert@cs.ucla.edu>
>>>     Reviewed-by: Sam James <sam@gentoo.org>
>>>     Cc: Jonathan Wakely <jwakely@redhat.com>
>>>     Cc: Szabolcs Nagy <nsz@port70.net>
>>>     Cc: Jakub Wilk <jwilk@jwilk.net>
>>>     Cc: A. Wilcox <AWilcox@wilcox-tech.com>
>>>     Signed-off-by: Alejandro Colomar <alx@kernel.org>
>>>
>>>
>>> BTW, Rich, please note the commits that this fixes: most of them are
>>> the initial commit that adds a page, which means that the function
>>> had always been documented with off64_t in the "spec".  Only splice(2)
>>> and copy_file_range(2) have been adjusted afterwards, and in a manner
>>> to be consistent with the rest of the pages, so I can only conclude
>>> that we didn't break the spec, but rather fixed it.
>>>
>>> Nevertheless, I'm sorry that it caused any problems to musl, and I'm
>>> happy that you reported them and so we can now improve the pages.
>>
>> While I like off_t, I am still unhappy that this seems to have been a
>> unilateral action from documentation side without even hearing input
>> from any major implementors other than myself.
> 
> Since Paul Eggert is a glibc maintainer, I consider his input as being
> representative enough of glibc, even with the list not in CC.  He
> usually produces very high quality patches, and I know he has special
> care about very odd platforms where type sizes are not usual.
> 
> We also had the review and approval of a distribution maintainer, Sam,
> so I wouldn't call this unilateral.
> 
> It's true we didn't ask the entire glibc list.  I'm fixing that by
> adding libc-alpha@ to the loop; let's see if they have anything to say
> about the patch, which BTW I pushed a few hours ago.  And hell, while
> we're at it, I'm CCing enh from bionic too, just in case he has any
> opinion (although he probably read this thread from the linux-man@
> list).
> 
> For future times, when opening a thread like this where input from
> glibc (or kernel) maintainers is not only welcome but essential, it's
> usually better to include the relevant list in CC right from the very
> first email, to provide them with full context, as I suggest in the
> CONTRIBUTING file.
> 
> And while at it, I'm thinking that maybe we should mention musl's
> list in CONTRIBUTING too.  Should I apply the patch below?  Would you
> mind reading that file, and suggesting anything you want for
> preventing similar conflicts with musl in the future?
> 
> Thank you all!
> Alex
> 
> diff --git a/CONTRIBUTING b/CONTRIBUTING
> index 80052c38e..a030b54a5 100644
> --- a/CONTRIBUTING
> +++ b/CONTRIBUTING
> @@ -27,12 +27,13 @@ Description
>         discussed in a man-pages email, please identify yourself as such.
>         Relevant mailing lists may include:
>  
>             Cc: LKML <linux-kernel@vger.kernel.org>
>             Cc: Linux API <linux-api@vger.kernel.org>
>             Cc: Glibc <libc-alpha@sourceware.org>
> +           Cc: musl libc <musl@lists.openwall.com>
>  
>         For other kernel mailing lists and maintainers, check the
>         <MAINTAINERS> file in the Linux kernel repository.
>  
>         Please don't send HTML email; it will be discarded by the list.
>  
> @@ -186,13 +187,13 @@ Description
>  Reporting bugs
>         Report bugs to the mailing list, following the instructions above
>         for sending mails to the list.  If you can write a patch (see
>         instructions for sending patches above), it's preferred.
>  
>         If you're unsure if the bug is in the manual page or in the code
> -       being documented (kernel, glibc, ...), it's best to send the
> +       being documented (kernel, libc, ...), it's best to send the
>         report to both at the same time, that is, CC all the mailing
>         lists that may be concerned by the report.
>  
>         Some distributions (for example Debian) apply patches to the
>         upstream manual pages.  If you suspect the bug is in one of those
>         patches, report it to your distribution maintainer.
> 
> 
>> Is "you can't use these
>> interfaces without -D_FILE_OFFSET_BITS=64" an acceptable outcome to
>> the glibc folks?
>>
>> Rich
> 

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


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

Re: [musl] Re: regression in man pages for interfaces using loff_t

[Rich Felker]

On Fri, Jun 30, 2023 at 12:11:20AM -0700, Paul Eggert wrote:
> On 2023-06-28 12:15, Rich Felker wrote:
> 
> >There's also the problem that off64_t is "exactly 64-bit" which makes
> >it unsuitable as an interface type for cross-platform functions where
> >one could imagine the native type being larger (rather horrifying but
> >possible).
> 
> Although we won't have files with 2**63 bytes any time soon, this is
> the best argument for preferring "loff_t" to "off64_t".
> 
> But come to think of it, it'd be better to document the type simply
> as "off_t", with a footnote saying the equivalent of "this assumes
> that on 32-bit glibc platforms you compile with
> -DFILE_OFFSET_BITS=64 like any sane person would." The intent really
> is off_t here, and that will remain true even if off_t ever widens
> past 64 bits.

I would love that these functions just take normal off_t. However,
glibc made it so off_t can be 32- or 64-bit depending on
_FILE_OFFSET_BITS, and if it's 32-bit, there is no matching version of
the libc syscall wrappers for these functions. It seems to have been a
conscious choice not to make any.

> I don't see why not. For example, POSIX-2018 requires int32_t even
> though C17 says it's optional. So there's precedent for POSIX
> adopting a type that's optional elsewhere.

That's the other way around and not related to what I was talking
about.

> Also, to POSIX loff_t is just as optional as off64_t is. glibc
> defines neither type if the app #defines _POSIX_C_SOURCE as POSIX
> requires. So from a standardization viewpoint there's no reason to
> prefer one type over the other.

From a standardization viewpoint, a more general standard does not
want to adopt things from a more specific standard that might have
hidden requirements or tie-in with other parts of the more specific
standard that the more general one would not want to be tied down to.

> >This is exactly the problem why ISO C is stuck with the
> >broken and unusable fseek/ftell that take long, and hasn't adopted
> >fseeko/ftello from POSIX -- their public interfaces use the
> >POSIX-governed type off_t, and as such, ISO C adopting them without
> >adopting the whole POSIX off_t is out of the question.
> 
> I'm not sure what the point is here, as far as standardization goes.
> Neither ISO C nor POSIX use loff_t, and neither is likely to ever
> use it: ISO C won't even adopt off_t much less loff_t, and POSIX
> works just fine with off_t and doesn't need loff_t. (The same goes
> for off64_t of course.)

Noobody's talking about if ISO C would adopt loff_t. I'm talking about
how an interface using a type that's under somebody else's
jurisdiction is problematic to adoption. loff_t having no precedent
except these Linux-introduced functions is why it's not problematic.

However I don't particular like or want to argue "for loff_t". What I
care about is that published interface definitions don't get changed
unilaterally by someone writing docs and made incompatible with
existing implementations.

If we want to work together for something mutually acceptable and
better than loff_t, that'd be fine with me too. But it seems
independent of fixing the immediate regression.

> But anyway this is all moot if we simply document the arguments as
> off_t with a footnote.

This is still changing the documentated signature, which isn't really
nice, and would not be compatible with glibc unless glibc went out of
its way to hide those functions when _FILE_OFFSET_BITS is 32.

Rich

[musl] Re: [libc-coord] Re: [musl] Re: regression in man pages for interfaces using loff_t

[Paul Eggert]

On 2023-06-30 16:37, Rich Felker wrote:
> glibc made it so off_t can be 32- or 64-bit depending on
> _FILE_OFFSET_BITS, and if it's 32-bit, there is no matching version of
> the libc syscall wrappers for these functions. It seems to have been a
> conscious choice not to make any.

Yes, _FILE_OFFSET_BITS=32 is obsolescent. Among other things in 
GNU/Linux it is guaranteed to stop working in the year 2038, because you 
can't have 64-bit time_t without also having 64-bit off_t. There is no 
interest in supporting _FILE_OFFSET_BITS=32 for new APIs, which these are.


> I'm talking about
> how an interface using a type that's under somebody else's
> jurisdiction

I don't understand why jurisdiction matters here. Although off_t is 
under someone else's (POSIX's) jurisdiction, that doesn't mean the Linux 
man pages can't use POSIX-specified types like off_t.


> This is still changing the documentated signature, which isn't really
> nice, and would not be compatible with glibc unless glibc went out of
> its way to hide those functions when _FILE_OFFSET_BITS is 32.

I don't see any incompatibility with glibc and the changes I proposed. 
The changes merely weaken the spec in the man pages in an area where the 
spec should be weakened. glibc is compatible with the spec before it was 
changed to use off64_t, it's compatible with the spec now that it uses 
off64_t, and it would continue to be compatible with the spec if the 
proposed changes are adopted.

[musl] Re: [libc-coord] Re: [musl] Re: regression in man pages for interfaces using loff_t

[Szabolcs Nagy]

* Paul Eggert <eggert@cs.ucla.edu> [2023-07-01 00:24:27 -0700]:
> On 2023-06-30 16:37, Rich Felker wrote:
> > This is still changing the documentated signature, which isn't really
> > nice, and would not be compatible with glibc unless glibc went out of
> > its way to hide those functions when _FILE_OFFSET_BITS is 32.
> 
> I don't see any incompatibility with glibc and the changes I proposed. The
> changes merely weaken the spec in the man pages in an area where the spec
> should be weakened. glibc is compatible with the spec before it was changed
> to use off64_t, it's compatible with the spec now that it uses off64_t, and
> it would continue to be compatible with the spec if the proposed changes are
> adopted.

loff_t * can be incompatible with off64_t * as well as off_t *.

the documentation change can break the api of an implementation,
it is not weakening the spec.

(it can also break abi if loff_t has different abi than off64_t. two
integer types can have same range, representation and syscall argument
passing abi, but different libc abi and different c++ abi)

i don't think you can claim that glibc is compatible either way, as
a future target port can define loff_t differently than off64_t.

[musl] Re: [libc-coord] Re: [musl] Re: regression in man pages for interfaces using loff_t

[Paul Eggert]

On 2023-07-01 06:36, Szabolcs Nagy wrote:

> loff_t * can be incompatible with off64_t * as well as off_t *.

loff_t * cannot be incompatible with the off64_t in the current 
implementation. The same goes for off_t, if you compile with 
_FILE_OFFSET_BITS set to 64.

As you suggest, a future implementation might change this. But if 
anything this strengthens the case for the documentation avoiding these 
pseudo-off_t types, as they're less stable.


> the documentation change can break the api of an implementation,
> it is not weakening the spec.

Are you talking about the doc change from loff_t to off64_t? If so, I 
agree that change invalidated the musl implementation. When I wrote "I 
don't see any incompatibility with glibc and the changes I proposed" I 
was talking about the patch proposed here:

https://lore.kernel.org/linux-man/31b53a8d-7cf4-b3a3-371f-a5723963383e@cs.ucla.edu/2-0001-off64_t-prefer-off_t-for-splice-etc.patch

As far as I can see, this proposed patch doesn't invalidate any 
implementation. If it does invalidate one could you please give an 
example call that follows the rules of the proposed patch but does not 
work on glibc or on musl?

Re: [musl] Re: [libc-coord] Re: [musl] Re: regression in man pages for interfaces using loff_t

[Rich Felker]

On Sat, Jul 01, 2023 at 12:24:27AM -0700, Paul Eggert wrote:
> On 2023-06-30 16:37, Rich Felker wrote:
> >glibc made it so off_t can be 32- or 64-bit depending on
> >_FILE_OFFSET_BITS, and if it's 32-bit, there is no matching version of
> >the libc syscall wrappers for these functions. It seems to have been a
> >conscious choice not to make any.
> 
> Yes, _FILE_OFFSET_BITS=32 is obsolescent. Among other things in
> GNU/Linux it is guaranteed to stop working in the year 2038, because
> you can't have 64-bit time_t without also having 64-bit off_t. There
> is no interest in supporting _FILE_OFFSET_BITS=32 for new APIs,
> which these are.

These are not "new APIs" from the standpoint of glibc, which already
had them in 32-bit-off_t API profiles and can't be expected just to
remove them.

I'm all for using off_t in new interfaces. But unless glibc folks
agree, I am not for redefining interface types in a way that breaks
one of their supported profiles any more than I am for redefining
interface types in the way that broke things with musl.

> >I'm talking about
> >how an interface using a type that's under somebody else's
> >jurisdiction
> 
> I don't understand why jurisdiction matters here. Although off_t is
> under someone else's (POSIX's) jurisdiction, that doesn't mean the
> Linux man pages can't use POSIX-specified types like off_t.

I don't know if I'm not communicating well here or what. This topic
was about why we don't use off64_t for these interfaces (because
off64_t is governed by LFS64) not a reason not to use off_t.

In explaining this I cited an analogy to why the fseeko/ftello
interfaces were doomed not to be accepted in ISO C (if POSIX had made
fseekll and ftellll using long long instead, there would have been a
clear path to putting them in C without pulling in POSIX types). That
has nothing to do with your proposal to just use off_t. Of course
using off_t in new custom interfaces that build on an underlying POSIX
base is fine and is the preferred way to do things.

> >This is still changing the documentated signature, which isn't really
> >nice, and would not be compatible with glibc unless glibc went out of
> >its way to hide those functions when _FILE_OFFSET_BITS is 32.
> 
> I don't see any incompatibility with glibc and the changes I
> proposed. The changes merely weaken the spec in the man pages in an
> area where the spec should be weakened. glibc is compatible with the
> spec before it was changed to use off64_t, it's compatible with the
> spec now that it uses off64_t, and it would continue to be
> compatible with the spec if the proposed changes are adopted.

That's simply not the case.

1. The spec was that apps declare loff_t objects and pass pointers to
   those to some of the interfaces. That works on all existing
   implementations and profiles.

2. The spec was silently changed to be that apps declare off64_t
   objects and pass those instead. This breaks on musl which
   intentionally does not have LFS64 types/interfaces.

3. Under your proposal, the spec is that apps declare off_t objects
   and pass pointers to those. This produces a pointer type mismatch,
   and buffer overflow, if building with glibc and (still default)
   _FILE_OFFSET_BITS=32.

Of course glibc could try to remedy this by somehow masking these
functions when _FILE_OFFSET_BITS=32 so they can't get used. If they
want to do that, great. However, the documentation does not specify a
particular glibc version, and if applications followed your proposed
change to the documentation, they would end up with dangerously broken
code when compiled on any existing glibc out in the wild with
_FILE_OFFSET_BITS=32.

This is why the only safe and reasonable thing to do, without an
extensive consensus process working to understand and assess the
impact of a change, is NOT TO MAKE CHANGES TO EXISTING INTERFACE
SPECIFICATIONS. It's really unsettling that this was done unilaterally
in such an important source of documentation as linux-man. Unless
glibc folks come up with a way to get on board with changing it to
off_t like you want, I do not want to get into another round of making
changes to "improve" something that was wrong about how the interface
was specified before. I just want to revert the breakage and establish
that this kind of breakage should not happen.

Rich

Re: [musl] Re: [libc-coord] Re: [musl] Re: regression in man pages for interfaces using loff_t

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 2577 bytes --]

On 7/1/23 16:32, Rich Felker wrote:
> This is why the only safe and reasonable thing to do, without an
> extensive consensus process working to understand and assess the
> impact of a change, is NOT TO MAKE CHANGES TO EXISTING INTERFACE
> SPECIFICATIONS. It's really unsettling that this was done unilaterally
> in such an important source of documentation as linux-man. Unless
> glibc folks come up with a way to get on board with changing it to
> off_t like you want, I do not want to get into another round of making
> changes to "improve" something that was wrong about how the interface
> was specified before. I just want to revert the breakage and establish
> that this kind of breakage should not happen.

Hi Rich,

Sorry for the trouble caused.  But let me clarify something.

What is the spec, and how did we deviate from it?  Are we (linux-man)
the spec?  Is it the kernel?  Is it glibc?  Probably, we are all.
We need to clarify that before accusing anyone of deviating from the
spec.

When there's disagreement between some of those 3 sources, what to do?

Here's what glibc's info page says (and I doubt I influenced into
modifying that):

  -- Function: ssize_t copy_file_range (int INPUTFD, off64_t *INPUTPOS,
           int OUTPUTFD, off64_t *OUTPUTPOS, ssize_t LENGTH, unsigned int
           FLAGS)

Here's what the kernel says:

$ grepc -tfsp copy_file_range
./include/linux/syscalls.h:986:
asmlinkage long sys_copy_file_range(int fd_in, loff_t __user *off_in,
				    int fd_out, loff_t __user *off_out,
				    size_t len, unsigned int flags);


Very often, the user-space wrapper disagrees with the types used by
the kernel, and in the manual pages we prioritize the wrappers.  In
many cases, I found that the manual pages were wrong (they stated a
type that was different from the glibc one), and fixed them for good.
In this case it seems the fix was wrong, and nobody noticed for a few
years.  Well, sorry for making this mistake.  I think overall, in the
last years I have fixed more stuff than I have broken.

In this case, I'm ready to fix this when you all agree.  Don't worry
too much about it.  I'm on vacation, so I'll come back to this thread in
a few days.

To me, reverting back to loff_t is fine if you all agree.  Also, it's
been an interesting thread about why we should avoid loff_t and
off64_t for new APIs.  I'll probably document some of those details.

Cheers,
Alex


-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


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

Re: [musl] Re: [libc-coord] Re: [musl] Re: regression in man pages for interfaces using loff_t

[Paul Eggert]

On 2023-07-01 07:32, Rich Felker wrote:

> These are not "new APIs" from the standpoint of glibc, which already
> had them in 32-bit-off_t API profiles and can't be expected just to
> remove them.

That's OK, as the proposed doc change doesn't remove the new APIs.

By "new APIs" I meant off_t-oriented APIs newer than the fallout from 
the LFS mess of the 1990s; these APIs are not that new to people versed 
with the Linux kernel.


> I'm all for using off_t in new interfaces. But unless glibc folks
> agree, I am not for redefining interface types in a way that breaks
> one of their supported profile

The change I just proposed doesn't invalidate any part of the glibc 
implementation, or of musl for that matter. They're both fine as-is. 
(The previous doc change - the one that prompted your objection - did 
invalidate the musl implementation, and that is one thing the proposed 
change fixes.)

Just to be clear, the proposed change is merely trying to document the 
current situation more carefully, so that programmers who are not expert 
in this area are better aware about the unlikely gotchas. It doesn't 
attempt to change or invalidate any existing implementation.


> 1. The spec was that apps declare loff_t objects and pass pointers to
>     those to some of the interfaces. That works on all existing
>     implementations and profiles.

The fact that it worked was merely an accident, as people didn't follow 
that old spec. None of the apps I know of use loff_t for these APIs.

The spec for these APIs should never have suggested using an oddball 
type like loff_t that actual app code typically doesn't use. The spec 
should instead suggest working code, code that is what people would or 
should actually write. The proposed patch does this.


> 2. The spec was silently changed to be that apps declare off64_t
>     objects and pass those instead. This breaks on musl which
>     intentionally does not have LFS64 types/interfaces.

The proposed patch fixes this.


> 3. Under your proposal, the spec is that apps declare off_t objects
>     and pass pointers to those. This produces a pointer type mismatch,
>     and buffer overflow, if building with glibc and (still default)
>     _FILE_OFFSET_BITS=32.

There's a loud compile-time error if you make such a mistake. It's the 
came sort of compile-time error you get if you misuse any glibc API by 
passing a wrong pointer type. If these compile-time errors are good 
enough for the rest of the glibc API, they're good enough here.


> Of course glibc could try to remedy this by somehow masking these
> functions when _FILE_OFFSET_BITS=32 so they can't get used.

I don't see why that is necessary, or even a good idea. Many (in my 
experience most) calls to these functions will work fine even with 
_FILE_OFFSET_BITS=32, because they're passing a null pointer. I don't 
see why we'd want to prohibit this.

[musl] Re: regression in man pages for interfaces using loff_t

[Szabolcs Nagy]

* Paul Eggert <eggert@cs.ucla.edu> [2023-06-28 11:21:39 -0700]:

> On 2023-06-28 10:53, Rich Felker wrote:
> > The whole reason loff_t exists is to avoid this problem and make a
> > type that's "always full width offset, regardless of _FILE_OFFSET_BITS
> > or _LARGEFILE64_SOURCE" to match with the kernel expectation for these
> > interfaces.
> 
> Why can't off64_t be that type, as it is in glibc? I'm not seeing why we
> need two names for the same type.

umm because off64_t is not a defined type?

https://godbolt.org/z/9sf6n8Y3e

Re: [musl] Re: regression in man pages for interfaces using loff_t

[Rich Felker]

On Wed, Jun 28, 2023 at 11:21:39AM -0700, Paul Eggert wrote:
> On 2023-06-28 10:53, Rich Felker wrote:
> >The whole reason loff_t exists is to avoid this problem and make a
> >type that's "always full width offset, regardless of _FILE_OFFSET_BITS
> >or _LARGEFILE64_SOURCE" to match with the kernel expectation for these
> >interfaces.
> 
> Why can't off64_t be that type, as it is in glibc? I'm not seeing
> why we need two names for the same type.

And to clarify one thing: nothing glibc is doing is wrong. It's fine
for glibc to define the function using off64_t as long as glibc's
underlying definitions of off64_t and loff_t are the same type. This
still matches the published documentation that says loff_t.

I'm not asking for any change by glibc here, just a revert to the man
page change which put a glibc implementation-detail in place of the
equivalent prior-documented public interface.

Rich