Skip to contentSkip to conversation

use C99 initialisers for PCI_VDEVICE()

PATCH In review Ulrich Eckhardt 2 participants 3 messages Apr 15, 2007

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
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

#2 lore raw
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=0

NAK. 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