Messages in this thread | | | Date | Fri, 19 Feb 2016 08:58:43 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() |
| |
* Peter Zijlstra <peterz@infradead.org> wrote:
> > Yeah, so please change this to something like: > > > > struct mcsafe_ret { > > u64 trap_nr; > > u64 bytes_left; > > }; > > > > this makes it crystal clear what the fields are about and what their unit is. > > Readability is king and modern consoles are wide enough, no need to abbreviate > > excessively. > > I prefer to use my modern console width to display multiple columns of text, > instead of wasting it to display mostly whitespace. Therefore I still very much > prefer ~80 char wide code.
Btw., the main reason I hate the col80 limit is that I see such patches frequently:
void pcibios_add_bus(struct pci_bus *bus) { +#ifdef CONFIG_DMI + const struct dmi_device *dmi; + struct dmi_dev_onboard *dslot; + char sname[128]; + + dmi = NULL; + while ((dmi = dmi_find_device(DMI_DEV_TYPE_DEV_SLOT, + NULL, dmi)) != NULL) { + dslot = dmi->device_data; + if (dslot->segment == pci_domain_nr(bus) && + dslot->bus == bus->number) { + dev_info(&bus->dev, "Found SMBIOS Slot %s\n", + dslot->dev.name); + snprintf(sname, sizeof(sname), "%s-%d", + dslot->dev.name, + dslot->instance); + pci_create_slot(bus, dslot->devfn, + sname, NULL); + } + } +#endif acpi_pci_add_bus(bus);
Which gobbledygook has 6 (!) col80 artifacts - and it's a pretty straightforward piece of code with just 2 levels of indentation.
It is IMHO much more readable in the following form:
void pcibios_add_bus(struct pci_bus *bus) { #ifdef CONFIG_DMI const struct dmi_device *dmi; struct dmi_dev_onboard *dslot; char sname[128];
dmi = NULL; while ((dmi = dmi_find_device(DMI_DEV_TYPE_DEV_SLOT, NULL, dmi)) != NULL) { dslot = dmi->device_data; if (dslot->segment == pci_domain_nr(bus) && dslot->bus == bus->number) { dev_info(&bus->dev, "Found SMBIOS Slot %s\n", dslot->dev.name); snprintf(sname, sizeof(sname), "%s-%d", dslot->dev.name, dslot->instance); pci_create_slot(bus, dslot->devfn, sname, NULL); } } #endif acpi_pci_add_bus(bus);
BYMMV.
Thanks,
Ingo
| |