linux-kernel - Re: [PATCH] drivers: pci: This patch fix the following checkpatch warning.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20190417213542.GU126710@google.com>
Date:   Wed, 17 Apr 2019 16:35:42 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Mohan Kumar <mohankumar718@...il.com>
Cc:     linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drivers: pci: This patch fix the following checkpatch
 warning.

Hi Mohan,

On Tue, Apr 16, 2019 at 07:43:40PM +0300, Mohan Kumar wrote:
> Use pr_err instead of printk

I don't mind taking patches like these, but they do tend to clutter
the revision history if we're not careful.  So here are some hints:

  - Make the subject line useful.  The one here really doesn't tell me
    anything.  Run "git log --oneline drivers/pci/bus.c" and follow
    the conventions there.  Something like:

      PCI: Convert printk(KERN_*) to pr_*()

  - Do all the similar changes at once.  I notice similar things in
    these files:

      drivers/pci/pci-acpi.c
      drivers/pci/pci-stub.c
      drivers/pci/pci-sysfs.c
      drivers/pci/pci.c
      drivers/pci/pcie/aspm.c
      drivers/pci/quirks.c
      drivers/pci/setup-bus.c
      drivers/pci/slot.c

    plus several in drivers/pci/hotplug/.  These could all be done in
    a single patch.

  - If there are several in a file that use the same prefix, add a
    "#define pr_fmt()".  This should be a separate patch so each patch
    does only one type of change, which makes them easier to review.

  - There are several uses of "printk(KERN_DEBUG)",
    "dev_printk(KERN_DEBUG)", and "pci_printk(KERN_DEBUG)".  The
    obvious thing would be to convert those to pr_debug(), dev_dbg(),
    and pci_dbg(), but I don't like that approach because pr_debug()
    and friends do different things depending on how the kernel is
    built.  The uses in PCI provide information that I want to appear
    in the dmesg log *always*.  So they could be (a) left alone or (b)
    converted from KERN_DEBUG to KERN_INFO (i.e., converted to
    pr_info(), dev_info(), etc).

> WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ...
> then dev_err(dev, ... then pr_err(...  to printk(KERN_ERR ...
> 
> Signed-off-by: Mohan Kumar <mohankumar718@...il.com>
> ---
>  drivers/pci/bus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 5cb40b2..2179a8b 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -23,7 +23,7 @@ void pci_add_resource_offset(struct list_head *resources, struct resource *res,
>  
>  	entry = resource_list_create_entry(res, 0);
>  	if (!entry) {
> -		printk(KERN_ERR "PCI: can't add host bridge window %pR\n", res);
> +		pr_err("PCI: can't add host bridge window %pR\n", res);
>  		return;
>  	}
>  
> -- 
> 2.7.4
>