[PATCH] Small initializer patch for ide-disk.c

[PATCH] Small initializer patch for ide-disk.c

[Art Haas]

Hi.

This patch adds C99 initializers to the file. It can clearly wait until
after 2.6.10 is released if you want to send it to Linus.

Art Haas

===== drivers/ide/ide-disk.c 1.109 vs edited =====
--- 1.109/drivers/ide/ide-disk.c	2004-11-06 19:55:34 -06:00
+++ edited/drivers/ide/ide-disk.c	2004-12-20 17:24:38 -06:00
@@ -871,11 +871,27 @@
 }
 
 static ide_proc_entry_t idedisk_proc[] = {
-	{ "cache",		S_IFREG|S_IRUGO,	proc_idedisk_read_cache,		NULL },
-	{ "geometry",		S_IFREG|S_IRUGO,	proc_ide_read_geometry,			NULL },
-	{ "smart_values",	S_IFREG|S_IRUSR,	proc_idedisk_read_smart_values,		NULL },
-	{ "smart_thresholds",	S_IFREG|S_IRUSR,	proc_idedisk_read_smart_thresholds,	NULL },
-	{ NULL, 0, NULL, NULL }
+	{
+		.name		= "cache",
+		.mode		= S_IFREG|S_IRUGO,
+		.read_proc	= proc_idedisk_read_cache,
+	 },
+	{
+		.name		= "geometry",
+		.mode		= S_IFREG|S_IRUGO,
+		.read_proc	= proc_ide_read_geometry,
+	},
+	{
+		.name		= "smart_values",
+		.mode		= S_IFREG|S_IRUSR,
+		.read_proc	= proc_idedisk_read_smart_values,
+	},
+	{
+		.name		= "smart_thresholds",
+		.mode		= S_IFREG|S_IRUSR,
+		.read_proc	= proc_idedisk_read_smart_thresholds,
+	},
+	{ .mode =  0 }
 };
 
 #else
-- 
Man once surrendering his reason, has no remaining guard against absurdities
the most monstrous, and like a ship without rudder, is the sport of every wind.

-Thomas Jefferson to James Smith, 1822

Re: [PATCH] Small initializer patch for ide-disk.c

[Jeff Garzik]

Art Haas wrote:
> Hi.
> 
> This patch adds C99 initializers to the file. It can clearly wait until
> after 2.6.10 is released if you want to send it to Linus.

The unpatched version is far more readable and maintainable.

	Jeff

Re: [PATCH] Small initializer patch for ide-disk.c

[John W. Linville]

On Tue, Dec 21, 2004 at 04:02:43PM -0500, Jeff Garzik wrote:
> Art Haas wrote:

> >This patch adds C99 initializers to the file. It can clearly wait until
 
> The unpatched version is far more readable and maintainable.

I dunno...other than the whitespace issue before the first "}",
I like the patch.

I think the new-style struct initializers have the advantage of
associating names with parameters, rather than relying on declaration
order to match a structure definition living elsewhere.

YMMV, of course...

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [PATCH] Small initializer patch for ide-disk.c

[Art Haas]

On Tue, Dec 21, 2004 at 04:02:43PM -0500, Jeff Garzik wrote:
> Art Haas wrote:
> >Hi.
> >
> >This patch adds C99 initializers to the file. It can clearly wait until
> >after 2.6.10 is released if you want to send it to Linus.
> 
> The unpatched version is far more readable and maintainable.
> 

I thought the patch made the file a little more readable is it removed
the 'NULL' values and the lines don't wrap. Regardless, the patch is
not critical, just cosmetic.

Art Haas

-- 
Man once surrendering his reason, has no remaining guard against absurdities
the most monstrous, and like a ship without rudder, is the sport of every wind.

-Thomas Jefferson to James Smith, 1822

Re: [PATCH] Small initializer patch for ide-disk.c

[Andrew Morton]

Jeff Garzik <jgarzik@pobox.com> wrote:
>
> Art Haas wrote:
> > Hi.
> > 
> > This patch adds C99 initializers to the file. It can clearly wait until
> > after 2.6.10 is released if you want to send it to Linus.
> 
> The unpatched version is far more readable and maintainable.
> 

I was thinking the exact opposite ;)

Re: [PATCH] Small initializer patch for ide-disk.c

[Jeff Garzik]

Andrew Morton wrote:
> Jeff Garzik <jgarzik@pobox.com> wrote:
> 
>>Art Haas wrote:
>>
>>>Hi.
>>>
>>>This patch adds C99 initializers to the file. It can clearly wait until
>>>after 2.6.10 is released if you want to send it to Linus.
>>
>>The unpatched version is far more readable and maintainable.
>>
> 
> 
> I was thinking the exact opposite ;)


The problem with this (and similar patches to PCI drivers' struct 
pci_device_id arrays) is that a single line explodes into 4-5 or more 
lines, when it is _already_ plainly obvious to the maintainer what each 
field value represents.

It uses more space without adding value to the maintainer ("I have to 
scroll a lot more to see the same information?  Thanks!").

Once the struct has exploded from one line per entry to bunches, the 
maintainer is then forced to reverse the damage by creating a macro that 
allows the data to shrink again.  Why not just avoid the expand-shrink 
cycle in the first place?  See example below, from drivers/ide/pci/piix.h.

Of course, Art's patch modifies drivers/ide/*, so Bart's opinion on the 
subject is far more important than mine.

	Jeff



#define DECLARE_PIIX_DEV(name_str) \
         {                                              \
                .name           = name_str,             \
                .init_setup     = init_setup_piix,      \
                .init_chipset   = init_chipset_piix,    \
                .init_hwif      = init_hwif_piix,       \
                .channels       = 2,                    \
                .autodma        = AUTODMA,              \
                .enablebits     = {{0x41,0x80,0x80}, {0x43,0x80,0x80}}, \
                .bootable       = ON_BOARD,             \
         }

Re: [PATCH] Small initializer patch for ide-disk.c

[Randy.Dunlap]

Jeff Garzik wrote:
> Andrew Morton wrote:
> 
>> Jeff Garzik <jgarzik@pobox.com> wrote:
>>
>>> Art Haas wrote:
>>>
>>>> Hi.
>>>>
>>>> This patch adds C99 initializers to the file. It can clearly wait until
>>>> after 2.6.10 is released if you want to send it to Linus.
>>>
>>>
>>> The unpatched version is far more readable and maintainable.
>>>
>>
>>
>> I was thinking the exact opposite ;)
> 
> 
> 
> The problem with this (and similar patches to PCI drivers' struct 
> pci_device_id arrays) is that a single line explodes into 4-5 or more 
> lines, when it is _already_ plainly obvious to the maintainer what each 
> field value represents.
> 
> It uses more space without adding value to the maintainer ("I have to 
> scroll a lot more to see the same information?  Thanks!").
> 
> Once the struct has exploded from one line per entry to bunches, the 
> maintainer is then forced to reverse the damage by creating a macro that 
> allows the data to shrink again.  Why not just avoid the expand-shrink 
> cycle in the first place?  See example below, from drivers/ide/pci/piix.h.
> 
> Of course, Art's patch modifies drivers/ide/*, so Bart's opinion on the 
> subject is far more important than mine.

I like to see the field names listed too.

But the real question to me (other than "will some old-style
initializers be deprecated by gcc in the future?") is
"who will read such initializers?".  I don't think that it
hurts readabiility by/for the maintainer(s) but it does help
readability for the rest of us.  IMHO obviously.

-- 
~Randy

Re: [PATCH] Small initializer patch for ide-disk.c

[Jeff Garzik]

On Tue, Dec 21, 2004 at 01:57:55PM -0800, Randy.Dunlap wrote:
> But the real question to me (other than "will some old-style
> initializers be deprecated by gcc in the future?") is

Note that the code in question did not use initializers at all,
so that's an unrelated issue.

The code was, and remains, compliant to C99 standards.

	Jeff

Re: [PATCH] Small initializer patch for ide-disk.c

[Randy.Dunlap]

Jeff Garzik wrote:
> On Tue, Dec 21, 2004 at 01:57:55PM -0800, Randy.Dunlap wrote:
> 
>>But the real question to me (other than "will some old-style
>>initializers be deprecated by gcc in the future?") is
> 
> 
> Note that the code in question did not use initializers at all,
> so that's an unrelated issue.
> 
> The code was, and remains, compliant to C99 standards.

It looks to me like it used unnamed initializers, not named ones,
and not the gcc-specific ones that we have a need to move away
from.

-- 
~Randy

Re: [PATCH] Small initializer patch for ide-disk.c

[Jeff Garzik]

Randy.Dunlap wrote:
> Jeff Garzik wrote:
> 
>> On Tue, Dec 21, 2004 at 01:57:55PM -0800, Randy.Dunlap wrote:
>>
>>> But the real question to me (other than "will some old-style
>>> initializers be deprecated by gcc in the future?") is
>>
>>
>>
>> Note that the code in question did not use initializers at all,
>> so that's an unrelated issue.
>>
>> The code was, and remains, compliant to C99 standards.
> 
> 
> It looks to me like it used unnamed initializers, not named ones,
> and not the gcc-specific ones that we have a need to move away
> from.

Sorry, that's what I meant, yes.

	Jeff

Re: [PATCH] Small initializer patch for ide-disk.c

[Bartlomiej Zolnierkiewicz]

On Tue, 21 Dec 2004 16:51:26 -0500, Jeff Garzik <jgarzik@pobox.com> wrote:
> Andrew Morton wrote:
> > Jeff Garzik <jgarzik@pobox.com> wrote:
> >
> >>Art Haas wrote:
> >>
> >>>Hi.
> >>>
> >>>This patch adds C99 initializers to the file. It can clearly wait until
> >>>after 2.6.10 is released if you want to send it to Linus.
> >>
> >>The unpatched version is far more readable and maintainable.
> >>
> >
> >
> > I was thinking the exact opposite ;)
> 
> The problem with this (and similar patches to PCI drivers' struct
> pci_device_id arrays) is that a single line explodes into 4-5 or more
> lines, when it is _already_ plainly obvious to the maintainer what each
> field value represents.
> 
> It uses more space without adding value to the maintainer ("I have to
> scroll a lot more to see the same information?  Thanks!").
> 
> Once the struct has exploded from one line per entry to bunches, the
> maintainer is then forced to reverse the damage by creating a macro that
> allows the data to shrink again.  Why not just avoid the expand-shrink
> cycle in the first place?  See example below, from drivers/ide/pci/piix.h.
>
> Of course, Art's patch modifies drivers/ide/*, so Bart's opinion on the
> subject is far more important than mine.

My opinion on this subject is the same as Jeff's.

Re: [PATCH] Small initializer patch for ide-disk.c

[Jeff Garzik]

(apologies for the duplicate message, if you are seeing this a second time)

Andrew Morton wrote:
> Jeff Garzik <jgarzik@pobox.com> wrote:
> 
>>Art Haas wrote:
>>
>>>Hi.
>>>
>>>This patch adds C99 initializers to the file. It can clearly wait until
>>>after 2.6.10 is released if you want to send it to Linus.
>>
>>The unpatched version is far more readable and maintainable.
>>
> 
> 
> I was thinking the exact opposite ;)


The problem with this (and similar patches to PCI drivers' struct 
pci_device_id arrays) is that a single line explodes into 4-5 or more 
lines, when it is _already_ plainly obvious to the maintainer what each 
field value represents.

It uses more space without adding value to the maintainer ("I have to 
scroll a lot more to see the same information?  Thanks!").

Once the struct has exploded from one line per entry to bunches, the 
maintainer is then forced to reverse the damage by creating a macro that 
allows the data to shrink again.  Why not just avoid the expand-shrink 
cycle in the first place?  See example below, from drivers/ide/pci/piix.h.

Of course, Art's patch modifies drivers/ide/*, so Bart's opinion on the 
subject is far more important than mine.

	Jeff



#define DECLARE_PIIX_DEV(name_str) \
         {                                              \
                .name           = name_str,             \
                .init_setup     = init_setup_piix,      \
                .init_chipset   = init_chipset_piix,    \
                .init_hwif      = init_hwif_piix,       \
                .channels       = 2,                    \
                .autodma        = AUTODMA,              \
                .enablebits     = {{0x41,0x80,0x80}, {0x43,0x80,0x80}}, \
                .bootable       = ON_BOARD,             \
         }