From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753285AbdCWWFt (ORCPT ); Thu, 23 Mar 2017 18:05:49 -0400 Received: from mail-vk0-f66.google.com ([209.85.213.66]:34909 "EHLO mail-vk0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751584AbdCWWFb (ORCPT ); Thu, 23 Mar 2017 18:05:31 -0400 MIME-Version: 1.0 X-Originating-IP: [108.49.102.27] In-Reply-To: <55f62ee9-9e98-f5e0-67eb-fc7aa5cbe8f8@schaufler-ca.com> References: <60ed4f02-4ff8-2ef2-bcc3-ae62bc61cda9@users.sourceforge.net> <55f62ee9-9e98-f5e0-67eb-fc7aa5cbe8f8@schaufler-ca.com> From: Paul Moore Date: Thu, 23 Mar 2017 18:05:29 -0400 Message-ID: Subject: Re: [PATCH 15/46] selinux: One check and function call less in genfs_read() after error detection To: Casey Schaufler Cc: SF Markus Elfring , linux-security-module@vger.kernel.org, selinux@tycho.nsa.gov, Eric Paris , James Morris , "Serge E. Hallyn" , Stephen Smalley , William Roberts , LKML , kernel-janitors@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 17, 2017 at 12:53 PM, Casey Schaufler wrote: > On 1/17/2017 8:37 AM, SF Markus Elfring wrote: >>>> @@ -2015,7 +2015,7 @@ static int genfs_read(struct policydb *p, void *fp) >>>> newgenfs = kzalloc(sizeof(*newgenfs), GFP_KERNEL); >>>> if (!newgenfs) { >>>> rc = -ENOMEM; >>>> - goto out; >>>> + goto exit; >>>> } >>>> >>>> rc = str_read(&newgenfs->fstype, GFP_KERNEL, fp, len); >>>> @@ -2101,7 +2101,7 @@ static int genfs_read(struct policydb *p, void *fp) >>>> kfree(newgenfs); >>>> } >>>> ocontext_destroy(newc, OCON_FSUSE); >>>> - >>>> +exit: >>>> return rc; >>> Why not replace the "goto out" with "return rc" rather >>> than add a target? >> Would you accept to use the statement "return -ENOMEM;" there instead? > > That would be even better. I *hate* code that does a jump to a label only to then do a return/exit. That said, see my earlier comments about not worrying too much about performance of the error path and in this case I like the "feel good" nature of the code where ever failure in the loop goes to "out". -- paul moore www.paul-moore.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Moore Date: Thu, 23 Mar 2017 22:05:29 +0000 Subject: Re: [PATCH 15/46] selinux: One check and function call less in genfs_read() after error detection Message-Id: List-Id: References: <60ed4f02-4ff8-2ef2-bcc3-ae62bc61cda9@users.sourceforge.net> <55f62ee9-9e98-f5e0-67eb-fc7aa5cbe8f8@schaufler-ca.com> In-Reply-To: <55f62ee9-9e98-f5e0-67eb-fc7aa5cbe8f8@schaufler-ca.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-security-module@vger.kernel.org On Tue, Jan 17, 2017 at 12:53 PM, Casey Schaufler wrote: > On 1/17/2017 8:37 AM, SF Markus Elfring wrote: >>>> @@ -2015,7 +2015,7 @@ static int genfs_read(struct policydb *p, void *fp) >>>> newgenfs = kzalloc(sizeof(*newgenfs), GFP_KERNEL); >>>> if (!newgenfs) { >>>> rc = -ENOMEM; >>>> - goto out; >>>> + goto exit; >>>> } >>>> >>>> rc = str_read(&newgenfs->fstype, GFP_KERNEL, fp, len); >>>> @@ -2101,7 +2101,7 @@ static int genfs_read(struct policydb *p, void *fp) >>>> kfree(newgenfs); >>>> } >>>> ocontext_destroy(newc, OCON_FSUSE); >>>> - >>>> +exit: >>>> return rc; >>> Why not replace the "goto out" with "return rc" rather >>> than add a target? >> Would you accept to use the statement "return -ENOMEM;" there instead? > > That would be even better. I *hate* code that does a jump to a label only to then do a return/exit. That said, see my earlier comments about not worrying too much about performance of the error path and in this case I like the "feel good" nature of the code where ever failure in the loop goes to "out". -- paul moore www.paul-moore.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: paul@paul-moore.com (Paul Moore) Date: Thu, 23 Mar 2017 18:05:29 -0400 Subject: [PATCH 15/46] selinux: One check and function call less in genfs_read() after error detection In-Reply-To: <55f62ee9-9e98-f5e0-67eb-fc7aa5cbe8f8@schaufler-ca.com> References: <60ed4f02-4ff8-2ef2-bcc3-ae62bc61cda9@users.sourceforge.net> <55f62ee9-9e98-f5e0-67eb-fc7aa5cbe8f8@schaufler-ca.com> Message-ID: To: linux-security-module@vger.kernel.org List-Id: linux-security-module.vger.kernel.org On Tue, Jan 17, 2017 at 12:53 PM, Casey Schaufler wrote: > On 1/17/2017 8:37 AM, SF Markus Elfring wrote: >>>> @@ -2015,7 +2015,7 @@ static int genfs_read(struct policydb *p, void *fp) >>>> newgenfs = kzalloc(sizeof(*newgenfs), GFP_KERNEL); >>>> if (!newgenfs) { >>>> rc = -ENOMEM; >>>> - goto out; >>>> + goto exit; >>>> } >>>> >>>> rc = str_read(&newgenfs->fstype, GFP_KERNEL, fp, len); >>>> @@ -2101,7 +2101,7 @@ static int genfs_read(struct policydb *p, void *fp) >>>> kfree(newgenfs); >>>> } >>>> ocontext_destroy(newc, OCON_FSUSE); >>>> - >>>> +exit: >>>> return rc; >>> Why not replace the "goto out" with "return rc" rather >>> than add a target? >> Would you accept to use the statement "return -ENOMEM;" there instead? > > That would be even better. I *hate* code that does a jump to a label only to then do a return/exit. That said, see my earlier comments about not worrying too much about performance of the error path and in this case I like the "feel good" nature of the code where ever failure in the loop goes to "out". -- paul moore www.paul-moore.com -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html