Re: [flashrom] [PATCH 1/2] convert programmer print messages to msg_p*

[Carl-Daniel Hailfinger]

On 31.03.2010 02:53, Sean Nelson wrote:
> Updated!

Thanks! I have a few comments, but this patch and its companion have
been in the queue for too long. Sorry for that. Please go ahead and
commit regardless of whether you address my comments or not.

It would be great if you could combine this patch and patch 2/2 into one
commit. That will allow us to tag 0.9.2 as r1000. Thanks.
> Signed-off-by: Sean Nelson

Acked-by: Carl-Daniel Hailfinger
> diff --git a/board_enable.c b/board_enable.c
> index 9b05f76..542d1ab 100644
> --- a/board_enable.c
> +++ b/board_enable.c
> @@ -1431,44 +1428,44 @@ static struct board_pciid_enable *board_match_coreboot_name(const char *vendor,
> if (!pci_dev_find(board->first_vendor, board->first_device))
> 	continue;
>
> if (board->second_vendor &&
>     !pci_dev_find(board->second_vendor, board->second_device))
> 	continue;
>
> if (vendor)
> 	return board;
>
> if (partmatch) {
> 	/* a second entry has a matching part name */
> 	printf("AMBIGUOUS BOARD NAME: %s\n", part);
> 	printf("At least vendors '%s' and '%s' match.\n",
> 	msg_pinfo("AMBIGUOUS BOARD NAME: %s\n", part);
> 	msg_pinfo("At least vendors '%s' and '%s' match.\n",
>        partmatch->lb_vendor, board->lb_vendor);
> 	printf("Please use the full -m vendor:part syntax.\n");
> 	msg_pinfo("Please use the full -m vendor:part syntax.\n");

The more I think about this one, the more I believe it should be _perr.
After all, this message is followed by an abort.
> 	return NULL;
>
> @@ -1488,27 +1485,27 @@ static struct board_pciid_enable *board_match_pci_card_ids(void)
>    					   board->second_device,
>    					   board->second_card_vendor,
>    					   board->second_card_device))
>    				continue;
>    		} else {
>    			if (!pci_dev_find(board->second_vendor,
>    					  board->second_device))
>    				continue;
>    		}
>    	}
> if (board->dmi_pattern) {
> 	if (!has_dmi_support) {
> 		fprintf(stderr, "WARNING: Can't autodetect %s %s,"
> 		msg_pinfo("WARNING: Can't autodetect %s %s,"

I know that I suggested to downgrade this to _pinfo, but you and Michael
were right that _perr is more appropriate because it will only be
triggered if a DMI match would be needed.
> 		       " DMI info unavailable.\n",
> 		       board->vendor_name, board->board_name);
> 		continue;