Re: [flashrom] [PATCH 2/2] convert general print messages to msg_g*

[Carl-Daniel Hailfinger]

On 31.03.2010 02:55, Sean Nelson wrote:
> Update to include only flashrom.c, since I agree that the other files
> don't exactly need changing.

Please combine this with the msg_p* patch into one commit. Thanks!
> Signed-off-by: Sean Nelson

Acked-by: Carl-Daniel Hailfinger 
Below are a few comments.
> diff --git a/flashrom.c b/flashrom.c
> index 2450dee..87ad066 100644
> --- a/flashrom.c
> +++ b/flashrom.c
> @@ -557,36 +557,36 @@ int check_erased_range(struct flashchip *flash, int start, int len)
> @return	0 for success, -1 for failure
>
> */
>  int verify_range(struct flashchip *flash, uint8_t *cmpbuf, int start, int len, char *message)
>  {
>    int i, j, starthere, lenhere, ret = 0;
>    int page_size = flash->page_size;
>    uint8_t *readbuf = malloc(page_size);
>    int failcount = 0;
>
> if (!len)
>    	goto out_free;
>
> if (!flash->read) {
> fprintf(stderr, "ERROR: flashrom has no read function for this flash chip.\n");
> msg_gerr("ERROR: flashrom has no read function for this flash chip.\n");

Sorry for not catching this earlier. This is a chip problem and should
be _cerr.
> @@ -816,64 +816,64 @@ int generate_testpattern(uint8_t *buf, uint32_t size, int variant)
>    		buf[i * 256 + 255] = i & 0xff;
>    	}
>    }
>
> return 0;
>  }
>
> int check_max_decode(enum chipbustype buses, uint32_t size)
>  {
>    int limitexceeded = 0;
>    if ((buses & CHIP_BUSTYPE_PARALLEL) &&
>        (max_rom_decode.parallel < size)) {
>    	limitexceeded++;
> printf_debug("Chip size %u kB is bigger than supported "
> msg_gdbg("Chip size %u kB is bigger than supported "

Difficult. It's a combined chip/programmer problem caused by a
programmer limitation. I'd say _pdbg, but you can leave it as you have
it. I just wanted to write this as a reminder for myself (or anyone else
who wants to go over the code again).
> 	     "size %u kB of chipset/board/programmer "
> 	     "for %s interface, "
> 	     "probe/read/erase/write may fail. ", size / 1024,
> 	     max_rom_decode.parallel / 1024, "Parallel");
>
> }
>    if ((buses & CHIP_BUSTYPE_LPC) && (max_rom_decode.lpc < size)) {
>    	limitexceeded++;
> printf_debug("Chip size %u kB is bigger than supported "
> msg_gdbg("Chip size %u kB is bigger than supported "

Ditto.
> 	     "size %u kB of chipset/board/programmer "
> 	     "for %s interface, "
> 	     "probe/read/erase/write may fail. ", size / 1024,
> 	     max_rom_decode.lpc / 1024, "LPC");
>
> }
>    if ((buses & CHIP_BUSTYPE_FWH) && (max_rom_decode.fwh < size)) {
>    	limitexceeded++;
> printf_debug("Chip size %u kB is bigger than supported "
> msg_gdbg("Chip size %u kB is bigger than supported "

Ditto.
> 	     "size %u kB of chipset/board/programmer "
> 	     "for %s interface, "
> 	     "probe/read/erase/write may fail. ", size / 1024,
> 	     max_rom_decode.fwh / 1024, "FWH");
>
> }
>    if ((buses & CHIP_BUSTYPE_SPI) && (max_rom_decode.spi < size)) {
>    	limitexceeded++;
> printf_debug("Chip size %u kB is bigger than supported "
> msg_gdbg("Chip size %u kB is bigger than supported "

Ditto.
> 	     "size %u kB of chipset/board/programmer "
> 	     "for %s interface, "
> 	     "probe/read/erase/write may fail. ", size / 1024,
> 	     max_rom_decode.spi / 1024, "SPI");
>
> }
>    if (!limitexceeded)
>    	return 0;
>    /* Sometimes chip and programmer have more than one bus in common,
>     * and the limit is not exceeded on all buses. Tell the user.
>     */
>    if (bitcount(buses) > limitexceeded)
>    	/* FIXME: This message is designed towards CLI users. */
> printf_debug("There is at least one common chip/programmer "
> msg_gdbg("There is at least one common chip/programmer "

Ditto.
> 	     "interface which can support a chip of this size. "
> 	     "You can try --force at your own risk.\n");
>
> return 1;
>  }
> @@ -914,78 +914,78 @@ struct flashchip *probe_flash(struct flashchip *first_flash, int force)
>    		goto notfound;
> if (first_flash == flashchips
>     || flash->model_id != GENERIC_DEVICE_ID)
> 	break;
>
> notfound:
>    	programmer_unmap_flash_region((void *)flash->virtual_memory, size);
>    }
>
> if (!flash || !flash->name)
>    	return NULL;
> printf("Found chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n",
> msg_cinfo("Found chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n",

I know I suggested _cinfo, but maybe we should split this message into a
_cinfo message (chip name, size, bus) and a _pinfo/_pdbg message
(address). Such a split might cause linebreak issues, so for now I think
_cinfo is good.
>       flash->vendor, flash->name, flash->total_size,
>       flashbuses_to_text(flash->bustype), base);

Regards,
Carl-Daniel