Am 25.12.2012 12:31 schrieb Stefan Tauner:
> To be able to get rid of lots of #ifdefs and centralize programmer-specific
> data more...
>  - introduce two new fields to struct programmer_entry, namely
>    enum type (OTHER, USB, PCI) and union devs (pcidev_status, usbdev_status
>    or char *note).
>  - use those fields to generate device listings in print.c and print_wiki.c.
>
> Bonus: add printing of USB devices to print_wiki.c and count supported PCI
> and USB devices.
>
> Signed-off-by: Stefan Tauner <[email protected]>

Thanks for your patch!

> diff --git a/flashrom.c b/flashrom.c
> index 77cae7c..b585dc4 100644
> --- a/flashrom.c
> +++ b/flashrom.c
> @@ -1576,6 +1622,35 @@ int selfcheck(void)
>                        * messages below without jumping through hoops. */
>                       continue;
>               }
> +             switch (p.type) {
> +             case USB:
> +                     if (p.devs.usb == NULL) {
> +                             msg_gerr("Programmer %s is of type USB, but its 
> device list is NULL!\n",
> +                                      p.name);
> +                             ret = 1;
> +                     }
> +                     break;
> +#if NEED_PCI == 1
> +             case PCI:
> +                     if (p.devs.pci == NULL) {
> +                             msg_gerr("Programmer %s is of type PCI, but its 
> device list is NULL!\n",
> +                                      p.name);
> +                             ret = 1;
> +                     }
> +                     break;
> +#endif
> +             case OTHER:
> +                     if (p.devs.note == NULL) {
> +                             if (strcmp("internal", p.name) == 0)
> +                                     break; // this is expected, no worry
> +                             msg_gerr("Programmer %s is of type OTHER, but 
> its note is NULL!\n", p.name);
> +                             ret = 1;
> +                     }
> +                     break;

Can't you simply handle USB/PCI/OTHER without switch()? After all,
devs.pci, devs.usb and devs.note are in a union and you can't detect
bugs like .type=USB combined with .devs.note="foobar" anyway. The only
special case is the internal programmer. Suggestion:

switch(p.type) {
case USB:
case PCI:
case OTHER:
    /* This is a union, check only one member. */
    if (p.devs.note == NULL) {
        if (strcmp("internal", p.name) == 0)
            break; /* This one has its device list stored separately. */
        msg_gerr("Programmer %s has neither a device list nor a textual
description!\n", p.name);
        ret = 1;
    break;
default:
...

 
> +             default:
> +                     msg_gerr("Programmer %s does not have a valid type 
> set!\n", p.name);
> +                     break;
> +             }
>               if (p.init == NULL) {
>                       msg_gerr("Programmer %s does not have a valid init 
> function!\n", p.name);
>                       ret = 1;

Rest looks ok.

Acked-by: Carl-Daniel Hailfinger <[email protected]>

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/


_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom

Reply via email to