Re: [PATCH 4/8] waitid(2): leave copyout of siginfo to syscall itself

[Al Viro]

On Mon, May 15, 2017 at 04:06:49PM -0700, Linus Torvalds wrote:
> On Mon, May 15, 2017 at 3:37 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > +struct waitid_info {
> > +       pid_t pid;
> > +       uid_t uid;
> > +       int status;
> > +       int why;
> > +};
> 
> Ugh. Could we please just name those with what they are actually used for?
> 
> Even if you hate the "si_" previx for some reason, I really don't see
> why we'd continue call it "why", when it's written to "si_code"
> 
> Yes, yes, I see the historical reason, and how "si_code" is just the
> low 16 bits of "why", and the high 16 bits is something else.

__SI_CHLD, and AFAICS it only matters for copy_siginfo_to_user() and its
relatives - basically, "how much of kernel-side struct siginfo do we have
initialized"...

> But now that there is a structure for that, could we not just make
> that explicit in the structure instead? Those games with "why" look
> really odd.

OK...

> So I can see why you'd like to keep this patch as "minimal
> conversion", but it would be really nice to have a followup patch that
> gets rid of the odd "why" games.

The thing is, we lack convenient defines for those constants.  We could
turn this "why" thing into u16 si_code, but then gcc will scream about
integer constant truncation ;-/  Suggestions?

BTW, I wonder if making those stores conditional is actually a win -
sure, for put_user() it used to be, but for plain stores...  Not sure.