From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751524AbdAPQmG (ORCPT ); Mon, 16 Jan 2017 11:42:06 -0500 Received: from mail-vk0-f67.google.com ([209.85.213.67]:36370 "EHLO mail-vk0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751487AbdAPQlB (ORCPT ); Mon, 16 Jan 2017 11:41:01 -0500 MIME-Version: 1.0 X-Originating-IP: [108.49.102.27] In-Reply-To: <1484580399.19104.6.camel@redhat.com> References: <1484580399.19104.6.camel@redhat.com> From: Paul Moore Date: Mon, 16 Jan 2017 11:40:59 -0500 Message-ID: Subject: Re: [PATCH 00/46] SELinux: Fine-tuning for several function implementations To: SF Markus Elfring Cc: Eric Paris , 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 Mon, Jan 16, 2017 at 10:26 AM, Eric Paris wrote: > Well, not totally irrelevant, I just reserve the right to ignore Eric if I disagree with him ;) > All of the patches look good to me except most of those which change > the handling of `rc=`. I have a personal style preference for > > rc = -ENOMEM; > val = kalloc(); > if (!val) > goto err; > > vs > > val = kalloc(); > if (!val) { > rc = -ENOMEM; > goto err; > } > > because it saves 1 line and I think the compiler does the right/same > thing. If there is preference among the people active in selinux > developers (like I said, I'm now irrelevant) I guess they win. My preference tends to be the other way around; I think putting the rc assignment in the if block makes the code more readable and that is how I tend to write things. That said, I don't recall ever requiring someone to redo a patch only because of this style nit ... if I did, shame on me. > But certainly a big +1 from me for the array allocation and sizeof() > changes. I do appreciate all the patches, thank you! However, a bit of a warning that it may take me a little bit of time to work my way through reviewing and merging all of them. -- paul moore www.paul-moore.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Moore Date: Mon, 16 Jan 2017 16:40:59 +0000 Subject: Re: [PATCH 00/46] SELinux: Fine-tuning for several function implementations Message-Id: List-Id: References: <1484580399.19104.6.camel@redhat.com> In-Reply-To: <1484580399.19104.6.camel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: SF Markus Elfring Cc: Eric Paris , 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 On Mon, Jan 16, 2017 at 10:26 AM, Eric Paris wrote: > Well, not totally irrelevant, I just reserve the right to ignore Eric if I disagree with him ;) > All of the patches look good to me except most of those which change > the handling of `rc=`. I have a personal style preference for > > rc = -ENOMEM; > val = kalloc(); > if (!val) > goto err; > > vs > > val = kalloc(); > if (!val) { > rc = -ENOMEM; > goto err; > } > > because it saves 1 line and I think the compiler does the right/same > thing. If there is preference among the people active in selinux > developers (like I said, I'm now irrelevant) I guess they win. My preference tends to be the other way around; I think putting the rc assignment in the if block makes the code more readable and that is how I tend to write things. That said, I don't recall ever requiring someone to redo a patch only because of this style nit ... if I did, shame on me. > But certainly a big +1 from me for the array allocation and sizeof() > changes. I do appreciate all the patches, thank you! However, a bit of a warning that it may take me a little bit of time to work my way through reviewing and merging all of them. -- paul moore www.paul-moore.com