Re: [PATCH] PCI: Fix general code style
From: Sergio Miguéns Iglesias <hidden>
Date: 2021-08-30 15:52:48
Also in:
linux-pci, lkml, xen-devel
I can not thank you enough for the amount of time you must have spent writing this response. I will look into those things in the following days for sure! ( I have already started looking into the "__ref" stuff) Thanks again for this, Sergio M. Iglesias. On 21/08/25 03:43, Bjorn Helgaas wrote:
On Thu, Aug 05, 2021 at 12:28:32AM +0200, Sergio Miguéns Iglesias wrote:quoted
The code style for most files was fixed. This means that blank lines were added when needed (normally after variable declarations), spaces before tabs were removed, some code alignment issues were solved, block comment style was fixed, every instance of "unsigned var" was replaced with "unsigned int var"... Etc. This commit does not change the logic of the code, it just fixes aesthetic problems.I generally *like* this, and it does fix some annoying things, but I think it's a little too much all at once. If we're working in a file and doing actual bug fixes or new functionality, and we want to fix some typos or something at the end, that might be OK, but I think the churn in the git history outweighs the benefit of this huge patch. So I would encourage you to use some of the PCI expertise you've gained by looking at all this code to work on something with a little more impact. Here are a couple ideas: - There are only two uses of __ref and __refdata in drivers/pci/. The fact that they're so rare makes me suspect that we don't need them. But I haven't investigated these to see. Somebody could check that out and remove them if we don't need them. Be aware that I will want a clear argument for why they're not needed :) - Coverity complains about several issues in drivers/pci/ [1]. Most of the time these are false positives, but not always. Sometimes there's an actual bug, and sometimes there's a way to restructure the code to avoid the warning (which usually means doing things the same way they are done elsewhere). - "make C=2 drivers/pci/" (sparse checker, [2]) complains about a few things. Leave the pci_power_t ones alone for now, but there are a couple other type issues that could be cleaned up. [1] https://docs.google.com/spreadsheets/d/19eyNDou83JACzf44j0NRzEWysva6g44G2_Z9IEXGVNk/edit?usp=sharing [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/dev-tools/sparse.rst?id=v5.13quoted
Signed-off-by: Sergio Miguéns Iglesias <redacted> --- drivers/pci/access.c | 22 +++++++++++++--------- drivers/pci/bus.c | 3 ++- drivers/pci/msi.c | 12 +++++++----- drivers/pci/pci-acpi.c | 3 ++- drivers/pci/pci-driver.c | 19 +++++++++++++------ drivers/pci/pci-sysfs.c | 14 ++++++++++++-- drivers/pci/pci.c | 16 ++++++++++++---- drivers/pci/proc.c | 15 +++++++++++++++ drivers/pci/quirks.c | 35 ++++++++++++++++++++++++----------- drivers/pci/remove.c | 1 + drivers/pci/rom.c | 2 +- drivers/pci/setup-bus.c | 5 ++++- drivers/pci/setup-irq.c | 12 +++++++----- drivers/pci/setup-res.c | 2 +- drivers/pci/slot.c | 5 ++++- drivers/pci/syscall.c | 5 +++-- drivers/pci/xen-pcifront.c | 20 ++++++++++++-------- 17 files changed, 133 insertions(+), 58 deletions(-)