From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by aws-us-west-2-korg-lkml-1.web.codeaurora.org (Postfix) with ESMTP id 967EBC433EF for ; Wed, 13 Jun 2018 06:23:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4EA7220693 for ; Wed, 13 Jun 2018 06:23:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4EA7220693 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=sony.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754482AbeFMGXI (ORCPT ); Wed, 13 Jun 2018 02:23:08 -0400 Received: from seldsegrel01.sonyericsson.com ([37.139.156.29]:1436 "EHLO SELDSEGREL01.sonyericsson.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754377AbeFMGXH (ORCPT ); Wed, 13 Jun 2018 02:23:07 -0400 Subject: Re: [PATCH 01/13] selinux: Cleanup printk logging in conditional To: Joe Perches , Paul Moore , Stephen Smalley , Eric Paris , James Morris , Daniel Jurgens , Doug Ledford , , , , "Serge E . Hallyn" References: <20180612080912.7827-1-peter.enderborg@sony.com> <20180612080912.7827-2-peter.enderborg@sony.com> From: peter enderborg Message-ID: Date: Wed, 13 Jun 2018 08:23:04 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Language: en-GB Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/12/2018 04:38 PM, Joe Perches wrote: > On Tue, 2018-06-12 at 10:09 +0200, Peter Enderborg wrote: >> Replace printk with pr_* to avoid checkpatch warnings. > I believe it would be nicer to remove the > "SELinux: " prefix embbeded in each format > and use a specific > > #define pr_fmt(fmt) "SELinux: " fmt > > to automatically prefix these formats. I cant argument about that, however some of the warnings and debug prints in this set does not have this so it will then change the actual output. (And I also think that they should have a the prefix, but I don't know why they don't) So I am not sure if it appropriate for a cleanup patch, it supposed to have no functional change. >> diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c > [] >> @@ -96,7 +96,7 @@ int evaluate_cond_node(struct policydb *p, struct cond_node *node) >> if (new_state != node->cur_state) { >> node->cur_state = new_state; >> if (new_state == -1) >> - printk(KERN_ERR "SELinux: expression result was undefined - disabling all rules.\n"); >> + pr_err("SELinux: expression result was undefined - disabling all rules.\n"); >> /* turn the rules on or off */ >> for (cur = node->true_list; cur; cur = cur->next) { >> if (new_state <= 0) > So, for instance, this patch could become: > (etc and so forth for each patch in this series) > > --- > security/selinux/ss/conditional.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c > index c91543a617ac..e96820d92b61 100644 > --- a/security/selinux/ss/conditional.c > +++ b/security/selinux/ss/conditional.c > @@ -7,6 +7,8 @@ > * the Free Software Foundation, version 2. > */ > > +#define pr_fmt(fmt) "SELinux: " fmt > + > #include > #include > #include > @@ -96,7 +98,7 @@ int evaluate_cond_node(struct policydb *p, struct cond_node *node) > if (new_state != node->cur_state) { > node->cur_state = new_state; > if (new_state == -1) > - printk(KERN_ERR "SELinux: expression result was undefined - disabling all rules.\n"); > + pr_err("expression result was undefined - disabling all rules\n"); > /* turn the rules on or off */ > for (cur = node->true_list; cur; cur = cur->next) { > if (new_state <= 0) > @@ -287,7 +289,7 @@ static int cond_insertf(struct avtab *a, struct avtab_key *k, struct avtab_datum > */ > if (k->specified & AVTAB_TYPE) { > if (avtab_search(&p->te_avtab, k)) { > - printk(KERN_ERR "SELinux: type rule already exists outside of a conditional.\n"); > + pr_err("type rule already exists outside of a conditional\n"); > goto err; > } > /* > @@ -302,7 +304,7 @@ static int cond_insertf(struct avtab *a, struct avtab_key *k, struct avtab_datum > node_ptr = avtab_search_node(&p->te_cond_avtab, k); > if (node_ptr) { > if (avtab_search_node_next(node_ptr, k->specified)) { > - printk(KERN_ERR "SELinux: too many conflicting type rules.\n"); > + pr_err("too many conflicting type rules\n"); > goto err; > } > found = 0; > @@ -313,13 +315,13 @@ static int cond_insertf(struct avtab *a, struct avtab_key *k, struct avtab_datum > } > } > if (!found) { > - printk(KERN_ERR "SELinux: conflicting type rules.\n"); > + pr_err("conflicting type rules\n"); > goto err; > } > } > } else { > if (avtab_search(&p->te_cond_avtab, k)) { > - printk(KERN_ERR "SELinux: conflicting type rules when adding type rule for true.\n"); > + pr_err("conflicting type rules when adding type rule for true\n"); > goto err; > } > } > @@ -327,7 +329,7 @@ static int cond_insertf(struct avtab *a, struct avtab_key *k, struct avtab_datum > > node_ptr = avtab_insert_nonunique(&p->te_cond_avtab, k, d); > if (!node_ptr) { > - printk(KERN_ERR "SELinux: could not insert rule.\n"); > + pr_err("could not insert rule\n"); > rc = -ENOMEM; > goto err; > } > @@ -387,12 +389,12 @@ static int cond_read_av_list(struct policydb *p, void *fp, struct cond_av_list * > static int expr_isvalid(struct policydb *p, struct cond_expr *expr) > { > if (expr->expr_type <= 0 || expr->expr_type > COND_LAST) { > - printk(KERN_ERR "SELinux: conditional expressions uses unknown operator.\n"); > + pr_err("conditional expressions uses unknown operator\n"); > return 0; > } > > if (expr->bool > p->p_bools.nprim) { > - printk(KERN_ERR "SELinux: conditional expressions uses unknown bool.\n"); > + pr_err("conditional expressions uses unknown bool\n"); > return 0; > } > return 1; > >