Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934841AbcCOQer (ORCPT ); Tue, 15 Mar 2016 12:34:47 -0400 Received: from h2.hallyn.com ([78.46.35.8]:38848 "EHLO h2.hallyn.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751048AbcCOQeo (ORCPT ); Tue, 15 Mar 2016 12:34:44 -0400 Date: Tue, 15 Mar 2016 11:34:36 -0500 From: "Serge E. Hallyn" To: Arnd Bergmann Cc: Serge Hallyn , David Howells , Yaowei Bai , James Morris , Andrew Morton , "Paul E. McKenney" , linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] cred/userns: define current_user_ns() as a function Message-ID: <20160315163435.GA14719@mail.hallyn.com> References: <1457992081-150281-1-git-send-email-arnd@arndb.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1457992081-150281-1-git-send-email-arnd@arndb.de> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2419 Lines: 64 On Mon, Mar 14, 2016 at 10:47:33PM +0100, Arnd Bergmann wrote: > The current_user_ns() macro currently returns &init_user_ns when > user namespaces are disabled, and that causes several warnings > when building with gcc-6.0 in code that compares the result of > the macro to &init_user_ns itself: > > fs/xfs/xfs_ioctl.c: In function 'xfs_ioctl_setattr_check_projid': > fs/xfs/xfs_ioctl.c:1249:22: error: self-comparison always evaluates to true [-Werror=tautological-compare] > if (current_user_ns() == &init_user_ns) > > This is a legitimate warning in principle, but here it isn't > really helpful, so I'm reprasing the definition in a way that > shuts up the warning. Apparently gcc only warns when comparing > identical literals, but it can figure out that the result of > an inline function can be identical to a constant expression > in order to optimize a condition yet not warn about the fact > that the condition is known at compile time. This is exactly > what we want here, and it looks reasonable because we generally > prefer inline functions over macros anyway. > > Signed-off-by: Arnd Bergmann Was a bit worried about the first hunk since capability.h doesn't include cred.h explicitly, but I guess if that was a problem it would've not compiled long ago due to current_cred_xxx(). Acked-by: Serge Hallyn > --- > include/linux/capability.h | 2 -- > include/linux/cred.h | 5 ++++- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/include/linux/capability.h b/include/linux/capability.h > index f314275d4e3f..00690ff92edf 100644 > --- a/include/linux/capability.h > +++ b/include/linux/capability.h > @@ -40,8 +40,6 @@ struct inode; > struct dentry; > struct user_namespace; > > -struct user_namespace *current_user_ns(void); > - > extern const kernel_cap_t __cap_empty_set; > extern const kernel_cap_t __cap_init_eff_set; > > diff --git a/include/linux/cred.h b/include/linux/cred.h > index 8d70e1361ecd..257db64562e5 100644 > --- a/include/linux/cred.h > +++ b/include/linux/cred.h > @@ -377,7 +377,10 @@ extern struct user_namespace init_user_ns; > #ifdef CONFIG_USER_NS > #define current_user_ns() (current_cred_xxx(user_ns)) > #else > -#define current_user_ns() (&init_user_ns) > +static inline struct user_namespace *current_user_ns(void) > +{ > + return &init_user_ns; > +} > #endif > > > -- > 2.7.0