Small initializer patch for ide-disk.c
PATCH 1/1 Small initializer patch for ide-disk.c
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 =====
drivers/ide/ide-disk.c +21 -5
@@ -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
Signature · 4 lines
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
Quoted 4 lines · 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.
The unpatched version is far more readable and maintainable.
Jeff
Quoted 9 lines · Jeff Garzik
Quoted 4 lines · 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.
The unpatched version is far more readable and maintainable.
I was thinking the exact opposite ;)
Quoted 6 lines · Andrew Morton
Quoted 4 lines · Jeff Garzik
Quoted 2 lines · Art Haas
Hi.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
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, \
}Quoted 20 lines · Jeff Garzik
Quoted 10 lines · Andrew Morton
Quoted 8 lines · Jeff Garzik
Quoted 4 lines · 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.
Quoted 5 lines
Quoted 2 lines
The unpatched version is far more readable and maintainable.
I was thinking the exact opposite ;)
Quoted 7 lines
The problem with this (and similar patches to PCI drivers' struct
pci_device_idarrays) 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.
Signature · 1 line
~Randy
Quoted 2 lines · Randy.Dunlap
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
Quoted 10 lines · Jeff Garzik
Quoted 2 lines · Randy.Dunlap
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.
Signature · 1 line
~Randy
Quoted 12 lines · Randy.Dunlap
Quoted 10 lines · Jeff Garzik
Quoted 2 lines · Randy.Dunlap
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.
Quoted 6 lines
Quoted 1 line
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
Quoted 24 lines · Jeff Garzik
Quoted 6 lines · Andrew Morton
Quoted 4 lines · Jeff Garzik
Quoted 2 lines · Art Haas
Hi.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
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_idarrays) 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!").
Quoted 7 lines
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.
(apologies for the duplicate message, if you are seeing this a second time)
Quoted 6 lines · Andrew Morton
Quoted 4 lines · Jeff Garzik
Quoted 2 lines · Art Haas
Hi.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
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, \
}Quoted 5 lines · Jeff Garzik
Quoted 1 line · Art Haas
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
Signature · 2 lines
John W. Linville [email protected]
Quoted 8 lines · Jeff Garzik
Quoted 4 lines · 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.
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
Signature · 4 lines
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