From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753757AbcBSH6u (ORCPT <rfc822;w@1wt.eu>);
	Fri, 19 Feb 2016 02:58:50 -0500
Received: from mail-wm0-f47.google.com ([74.125.82.47]:38179 "EHLO
	mail-wm0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1750805AbcBSH6t (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 19 Feb 2016 02:58:49 -0500
Date: Fri, 19 Feb 2016 08:58:43 +0100
From: Ingo Molnar <mingo@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Tony Luck <tony.luck@intel.com>, linux-kernel@vger.kernel.org,
        Thomas Gleixner <tglx@linutronix.de>, "H. Peter Anvin" <hpa@zytor.com>,
        Borislav Petkov <bp@alien8.de>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v11 3/4] x86, mce: Add __mcsafe_copy()
Message-ID: <20160219075843.GA7854@gmail.com>
References: <cover.1455732970.git.tony.luck@intel.com>
 <be86a03141b015d5a5e9477849585a62d4635308.1455732970.git.tony.luck@intel.com>
 <20160218082107.GA17851@gmail.com>
 <20160218095905.GC25010@twins.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160218095905.GC25010@twins.programming.kicks-ass.net>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


* 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