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