[flashrom] [PATCH 2/4] Refactor PCI and USB device status printing.

[Carl-Daniel Hailfinger]

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 <stefan.tauner at student.tuwien.ac.at>

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 <c-d.hailfinger.devel.2006 at gmx.net>

Regards,
Carl-Daniel

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