On Mon, Dec 18, 2023 at 7:58 PM Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxx> wrote: > > 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. > Makes sense. Yes, we do have flags for all commands accepting token FD, except for one, BPF_BTF_LOAD, but it's trivial to add flags there as well. I'll prepare a patch. > 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
