use C99 initialisers for PCI_VDEVICE()
PATCH 1/1 use C99 initialisers for PCI_VDEVICE()
(Note: CC me please, I'm not subscribed.)
Not much to say about the patch (it's against 2.6.20.6), it just converts a macro to generate C99-style initialisers.
linux/pci.h (working copy) renamed from linux/pci.h (revision 17) +4 -3
@@ -407,9 +407,10 @@ * private data. */ -#define PCI_VDEVICE(vendor, device) \ - PCI_VENDOR_ID_##vendor, (device), \ - PCI_ANY_ID, PCI_ANY_ID, 0, 0 +#define PCI_VDEVICE(vend, dev) \ + .vendor=PCI_VENDOR_ID_##vend, .device=(dev), \ + .subvendor=PCI_ANY_ID, .subdevice=PCI_ANY_ID, \ + .class=0, .class_mask=0
However, I still have two issues with this:
1. It explicitly allows in the comments for PCI_VDEVICE to have non-C99 fields follow for the driver_data field. IMHO this is wrong per se (just as old-style initialisers are), so the comment should perhaps be removed.
2. What happens to fields not initialised with this? I believe that these are initialised with zero, just like missing fields in old initialisers are, right? In that case, I would remove the initialisers for class and class_mask, so that people can at least optionally use them. However, this goes hand in hand with issue #1 because it would definitely break code that lets old-style initialisers follow.
As far as issue #2 is concerned, I did some checking (grep -r PCI_VDEVICE) and the only places where this is at all used is in the ATA code (drivers/ata)! Hmmm, no problem, that is grunt work but easily patched, too. If consensus exists that the class/class_mask fields should be removed, I hereby volunteer to submit a patch for that and the ATA code.
Uli
Quoted 7 lines · Ulrich Eckhardt
(Note: CC me please, I'm not subscribed.)
Not much to say about the patch (it's against 2.6.20.6), it just converts a macro to generate C99-style initialisers.
linux/pci.h (working copy) line 413
Quoted 10 lines
linux/pci.h (working copy) renamed from linux/pci.h (revision 17) +4
-3@@ -407,9 +407,10 @@ * private data. */ -#define PCI_VDEVICE(vendor, device) \ - PCI_VENDOR_ID_##vendor, (device), \ - PCI_ANY_ID, PCI_ANY_ID, 0, 0 +#define PCI_VDEVICE(vend, dev) \ + .vendor=PCI_VENDOR_ID_##vend, .device=(dev), \ + .subvendor=PCI_ANY_ID, .subdevice=PCI_ANY_ID, \ + .class=0, .class_mask=0
NAK. C99 initializers were intentionally avoided. Look at the usage of this macro.
Jeff
Quoted 2 lines · Jeff Garzik
Quoted 1 line · Ulrich Eckhardt
(Note: CC me please, I'm not subscribed.)
[...]
Quoted 10 lines
linux/pci.h (working copy) line 413
Quoted 7 lines
linux/pci.h (working copy) renamed from linux/pci.h (revision 17) +4
-3-#define PCI_VDEVICE(vendor, device) \ - PCI_VENDOR_ID_##vendor, (device), \ - PCI_ANY_ID, PCI_ANY_ID, 0, 0 +#define PCI_VDEVICE(vend, dev) \ + .vendor=PCI_VENDOR_ID_##vend, .device=(dev), \ + .subvendor=PCI_ANY_ID, .subdevice=PCI_ANY_ID, \ + .class=0, .class_mask=0NAK. C99 initializers were intentionally avoided. Look at the usage of this macro.
I'm aware of the uses and documentation of this, I was just under the impression that C99 initialisers were the agreed way to go because of their resistance against reording of fields. Of course, if that assumption is wrong, this patch is moot anyway.
Am I generally wrong or is this case an intentional exception?
Uli