From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753148AbdEOXqi (ORCPT ); Mon, 15 May 2017 19:46:38 -0400 Received: from zeniv.linux.org.uk ([195.92.253.2]:35870 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753121AbdEOXqg (ORCPT ); Mon, 15 May 2017 19:46:36 -0400 Date: Tue, 16 May 2017 00:46:33 +0100 From: Al Viro To: Linus Torvalds Cc: Linux Kernel Mailing List , Ingo Molnar , Oleg Nesterov , Peter Zijlstra , Christoph Hellwig Subject: Re: [PATCH 4/8] waitid(2): leave copyout of siginfo to syscall itself Message-ID: <20170515234633.GN390@ZenIV.linux.org.uk> References: <20170515223157.GM390@ZenIV.linux.org.uk> <20170515223716.2085-1-viro@ZenIV.linux.org.uk> <20170515223716.2085-4-viro@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.8.0 (2017-02-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, May 15, 2017 at 04:06:49PM -0700, Linus Torvalds wrote: > On Mon, May 15, 2017 at 3:37 PM, Al Viro 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.