From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932823AbcFBONp (ORCPT ); Thu, 2 Jun 2016 10:13:45 -0400 Received: from h2.hallyn.com ([78.46.35.8]:35386 "EHLO h2.hallyn.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932126AbcFBONo (ORCPT ); Thu, 2 Jun 2016 10:13:44 -0400 Date: Thu, 2 Jun 2016 09:13:49 -0500 From: "Serge E. Hallyn" To: Rui Teng Cc: serge.hallyn@canonical.com, james.l.morris@oracle.com, serge@hallyn.com, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, Andy Lutomirski Subject: Re: [PATCH] security: Use || instead of | for boolean expressions Message-ID: <20160602141349.GA26954@mail.hallyn.com> References: <1464760982-3721-1-git-send-email-rui.teng@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1464760982-3721-1-git-send-email-rui.teng@linux.vnet.ibm.com> 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 On Wed, Jun 01, 2016 at 02:03:02PM +0800, Rui Teng wrote: > Sparse spits out the following warning: > security/commoncap.c:989:41: warning: dubious: !x | y > > Bitwise and logical are equivalent here, but logical was intended. > Replacing the bit-wise '|' with the boolean '||' silences the sparse warning. Hi, this looks ok, but I'm worried by > The generated code for both cases is the same. That cannot be. The logical result should be the same, but the generated code cannot be. I'm cc:ing Andy as this code came in with his patch. Is there an actual reason for having used bitwise here? thanks, -serge > Signed-off-by: Rui Teng > --- > security/commoncap.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/security/commoncap.c b/security/commoncap.c > index e7fadde..8f6fb24 100644 > --- a/security/commoncap.c > +++ b/security/commoncap.c > @@ -976,7 +976,7 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3, > > case PR_CAP_AMBIENT: > if (arg2 == PR_CAP_AMBIENT_CLEAR_ALL) { > - if (arg3 | arg4 | arg5) > + if (arg3 || arg4 || arg5) > return -EINVAL; > > new = prepare_creds(); > @@ -986,7 +986,7 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3, > return commit_creds(new); > } > > - if (((!cap_valid(arg3)) | arg4 | arg5)) > + if (((!cap_valid(arg3)) || arg4 || arg5)) > return -EINVAL; > > if (arg2 == PR_CAP_AMBIENT_IS_SET) { > -- > 2.7.4