Re: [musl] [PATCH 1/2] V3 resubmitting old statx patch with changes

[Markus Wichmann <nullplan@gmx.net>]

Am Wed, Apr 24, 2024 at 07:55:32PM -0400 schrieb lolzery wowzery:
> Hi all! I'm new to git over email, new to musl, and I want to really
> get into contributing to musl. So, I'd really appreciate any
> tips/comments/info for a newbie like me!
>

Here's one: Please don't top-post. Another one: Are you the same as the
OP in this thread? Because I don't seem to have that message, but Rich
was quoting someone named "Duncan Bellamy", and that is not the name you
put on this message.

> Please do not merge these changes yet. There's numerous problems with
> this code above and beyond the issues you pointed out, and each of
> those issues led me down successive rabbit holes all over the musl
> source code and I've ended up opening a sizable can of worms,
> including io race condition bugs in fallback conditions, symbol
> weakness problems, header/struct misorganization, and bugs in syscall
> support detection making musl silently/unexpectedly fail under certain
> seccomp configurations.
>

This is called scope creep, and you should not do it. Fix one issue at a
time. If there are problems, report them or send patches one at a time.
Especially when you don't know what to do, you might find that the issue
you are worried about is not a problem at all when you just speak to
people about it.

> Also, let me cite the earlier patch from tony.ambardar@gmail.com about
> renameat2. My patch will include a full proper renameat2
> implementation with validation and fallback and will compile on
> systems where SYS_renameat2 is undefined (using exclusively the
> fallback.)
>

SYS_renameat2 is defined in every architecture musl supports. You have
created code that will not be used. I don't think this is a good thing.

Generally, new system calls are added to all architectures, though it
may take time. Therefore fallbacks for them not being defined are
typically useless. And renameat2 is older than seccomp, so not even
particularly new.

Ciao,
Markus