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

[Alejandro Colomar <alx@kernel.org>]

[-- 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 --]