[patch] use C99 initialisers for PCI_VDEVICE()

[patch] use C99 initialisers for PCI_VDEVICE()

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

--- include/linux/pci.h   (revision 17)
+++ include/linux/pci.h   (working copy)
@@ -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

Re: [patch] use C99 initialisers for PCI_VDEVICE()

[Jeff Garzik]

Ulrich Eckhardt wrote:
> (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.
> 
> --- include/linux/pci.h   (revision 17)
> +++ include/linux/pci.h   (working copy)
> @@ -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

Re: [patch] use C99 initialisers for PCI_VDEVICE()

[Ulrich Eckhardt]

On Sunday 15 April 2007 14:53, Jeff Garzik wrote:
> Ulrich Eckhardt wrote:
> > (Note: CC me please, I'm not subscribed.)
[...]
> > -#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