Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1337999
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() |
| Date | 2016-02-19 09:50 +0100 |
| Message-ID | <r3OVe-2o2-57@gated-at.bofh.it> (permalink) |
| References | <r3f1n-1O1-3@gated-at.bofh.it> <r3f1p-1O1-45@gated-at.bofh.it> <r3s8j-2Pa-19@gated-at.bofh.it> <r3txo-3CY-19@gated-at.bofh.it> <r3O8O-1Lc-7@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Fri, Feb 19, 2016 at 08:58:43AM +0100, Ingo Molnar wrote:
> > 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.
So I mostly agree, although your example does wrap on my normal display
width. The thing is though, we have to have a limit, otherwise people
will completely let loose and we'll end up with lines >200 chars wide
(I've worked on code like that in the past, and its a right pain).
And 80 has so far mostly worked just fine. Its just that people seem
unable to take guidelines as just than, and instead produce the most
horrible code just to adhere to checkpatch or whatnot.
And I'd much rather have an extra column of code than waste a lot of
screen-estate to display mostly whitespace.
Also, this 'artificial' limit on indentation level does in general
encourage people to write saner code.
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v11 0/4] Machine check recovery when kernel accesses poison Tony Luck <tony.luck@intel.com> - 2016-02-17 19:30 +0100
[PATCH v11 4/4] x86: Create a new synthetic cpu capability for machine check recovery Tony Luck <tony.luck@intel.com> - 2016-02-17 19:30 +0100
[tip:x86/asm] x86/cpufeature: Create a new synthetic cpu capability for machine check recovery tip-bot for Tony Luck <tipbot@zytor.com> - 2016-02-18 11:30 +0100
[PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Tony Luck <tony.luck@intel.com> - 2016-02-17 19:30 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Ingo Molnar <mingo@kernel.org> - 2016-02-18 09:30 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Peter Zijlstra <peterz@infradead.org> - 2016-02-18 11:00 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Borislav Petkov <bp@alien8.de> - 2016-02-18 11:30 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Ingo Molnar <mingo@kernel.org> - 2016-02-18 11:30 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Peter Zijlstra <peterz@infradead.org> - 2016-02-18 11:40 +0100
RE: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() "Luck, Tony" <tony.luck@intel.com> - 2016-02-18 16:00 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Ingo Molnar <mingo@kernel.org> - 2016-02-19 09:00 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Peter Zijlstra <peterz@infradead.org> - 2016-02-19 09:50 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Ingo Molnar <mingo@kernel.org> - 2016-02-19 11:00 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Ingo Molnar <mingo@kernel.org> - 2016-02-18 11:40 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Borislav Petkov <bp@alien8.de> - 2016-02-18 11:40 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Ingo Molnar <mingo@kernel.org> - 2016-02-18 19:50 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Borislav Petkov <bp@alien8.de> - 2016-02-18 11:40 +0100
[PATCH v12] x86, mce: Add memcpy_trap() "Luck, Tony" <tony.luck@intel.com> - 2016-02-18 22:20 +0100
Re: [PATCH v12] x86, mce: Add memcpy_trap() Ingo Molnar <mingo@kernel.org> - 2016-02-19 10:20 +0100
Re: [PATCH v13] x86, mce: Add memcpy_trap() "Luck, Tony" <tony.luck@intel.com> - 2016-02-19 19:00 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Linus Torvalds <torvalds@linux-foundation.org> - 2016-02-18 19:20 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() "Luck, Tony" <tony.luck@intel.com> - 2016-02-18 20:00 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Ingo Molnar <mingo@kernel.org> - 2016-02-18 21:20 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Dan Williams <dan.j.williams@intel.com> - 2016-02-18 22:40 +0100
Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy() Ingo Molnar <mingo@kernel.org> - 2016-02-18 20:00 +0100
csiph-web