Re: pull-request: bpf-next 2023-12-18

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 18 Dec 2023 at 17:48, Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
>
> Point taken.
> We can do s/__u32 token_fd/__u64 token/
> and waste upper 32-bit as flags that indicate that lower 32-bit is an FD
> or
> are you ok with __u32 token that is 'fd + 1'.

No, you make it follow the standard pattern that Unix has always had:
file descriptors are _signed_ integer, and negative means error (or
special cases).

Now, traditionally a 'fd' is literally just of type "int", but for
structures it's actually good to make it be a sized entity, so just
make it be __s32, and make any special cases be actual negative
numbers.

Because I'll just go out on a limb and say that two billion file
descriptors is enough for anybody, and if we ever were to hit that
number, we'll have *way* more serious problems elsewhere long long
before. And in practice, "int" is 32-bit on all current and
near-future architectures, so "__s32" really is the same as "int" in
all practical respects, and making the size explicit is just a good
idea.

You might want to perhaps pre-reserve a few negative numbers for
actual special cases, eg "openat()" uses

    #define AT_FDCWD -100

which I don't think is a great example to follow in the details: it
should have parenthesis, and "100" is a rather odd number to choose,
but it's certainly an example of a not-fundamentally-broken "not a
file descriptor, but a special case".

Now, if you have a 'flags' or 'cmd' field for *other* reasons, then
you can certainly just use one of the flags for "I have a file
descriptor". But don't do some odd "translate values", and don't add
32 bits just for that.

That's also a perfectly fine traditional unix use (example: socket
control messages - "struct cmsghdr" with "cmsg_type = SCM_RIGHTS" in
unix domain sockets).

But if you don't have some other reason for having a separate flag for
"I also have a file descriptor you should use", then just make a
negative number mean "no file descriptor".

It's easy to test for the number being negative, but it's also just
easy to *not* test for, ie it's also perfectly fine to just do
something like

        struct fd f = fdget(fd);

without ever even bothering to test whether 'fd' is negative or not.
It is guaranteed to fail for negative numbers and just look exactly
like the "not open" case, so if you don't care about the difference
between "invalid" and "not open", then a negative fd also works just
as-is with no extra code at all.

                   Linus

                     Linus




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux