Re: [GIT PULL] pid: use flex array

[Christian Brauner]

On Fri, Jun 30, 2023 at 12:12:22AM -0700, Linus Torvalds wrote:
> On Thu, 29 Jun 2023 at 23:51, Christian Brauner <brauner@xxxxxxxxxx> wrote:
> >
> > I have no preference for either syntax. Both work. But this is probably
> > more an objection to this being mixed in with the flex array change in
> > the first place.
> 
> Yes. I looked at it, and tried to figure out if it was related
> somehow, and decided that no, it can't possibly be, and must be just
> an unrelated change.

Yeah, I admit that I would've paid more attention to this detail if it
would've been in a fs/ codepath. So that's fully on me.

> 
> > I did react to that in the original review here:
> > https://lore.kernel.org/all/20230518-zuneigen-brombeeren-0a57cd32b1a7@brauner
> > but then I grepped for it and saw it done in a few other places already
> 
> Yeah, we do end up growing new uses of 'use 0 as a pointer' almost as
> quickly as we get rid of them.
> 
> We got rid of a couple just recently in commit dadeeffbe525 ("fbdev:
> hitfb: Use NULL for pointers"), but yes, a quick
> 
>     git grep '\*)0\>'
> 
> shows many more.
> 
> And some of them are even ok. I don't think it's always wrong,
> particularly if you then abstract it out.
> 
> So doing something like that
> 
>    #define PCI_IOBASE ((void __iomem *)0)
> 
> makes perfect sense. It's literally abstracting out something real (in
> this case yes, it looks like a NULL pointer, but it's actually a
> pointer with a strict type that just happens to have the value zero.
> 
> So that "NULL pointer with a type" concept makes sense, but it really
> should be abstracted out, not be in the middle of some random code.
> 
> And that's *particularly* true when we already have the exact
> abstraction for the situation that the code then uses (ie in this case
> that "struct_size_t()" thing). Writing it out - in an ugly form -
> using the disgusting traditional "constant integer zero can be cast to
> any pointer" thing - just makes me go "Eugghhh!".
> 
> I mean, if this ugly part was just a small detail in a l;arger patch,
> I probably would just have let it slide. It works. It is what it is.
> 
> But when three quarters of the patch was stuff I found questionable, I
> just couldn't stomach it.
> 
> I'm looking at some of the grep hits, and I'm going "ok, that's a C++
> programmer". I think there's a very real reason why so many of them
> are in bpf code and in the bpf tests.
> 
> C++ made a *huge* fundamental design mistake early on wrt NULL, and a
> generation of C++ programmers were poisoned by it and you had people
> who swore until they were blue that 0 and NULL had to be the same
> thing.
> 
> They were horribly and utterly wrong. But sometimes when it takes you
> three decades to admit that you were wrong all that time, it's just
> too painful to admit.
> 
> There are literally people who still can't admit to their mistake, and
> refuse to use NULL, and use either 0 or 'nullptr'.

I have to admit that I've never touched C++ in my life in any meaningful
way. It was C, Go, and then Rust...

I've grepped around a bit and I saw that the
struct_size((struct bla *)NULL, ...)
pattern seems to be used in most places that have similar needs. Not
sure if there's something nicer.

I gave this thing a stab myself since I have a few minutes and so Kees
doesn't have to do it. Authorship retained and dropped the ack. Is the
following more acceptable?