[flashrom] [PATCH] dummy programmer: blacklist SPI commands

[David Hendricks]

Overall looks good to me. I tested it out by blacklisting read, rdid, write,
wren, and the chip erase commands in various combinations and it seems to
work reasonably well. SPI opcode parsing seemed to work fine as well.

For others interested in giving this a try, here is another example of
syntax (courtesy of Carl-Danial on IRC) for us laymen: flashrom -p
dummy:emulate=SST25VF032B,image=dummy_simulator.rom,spi_blacklist=0x03

I don't know man page formatting well, so I didn't review that part. As far
as the code, I say:
Acked-by: David Hendricks <dhendrix at google.com>

Minor comments below, though I would be satisfied if the code were committed
as-is.

On Thu, Jan 20, 2011 at 5:36 PM, Maciej Pijanka <maciej.pijanka at gmail.com>wrote:

> > Index: flashrom-emulate_spi_flashchip_command_blacklist/dummyflasher.c
> >
> >
> > +     tmp = extract_programmer_param("spi_blacklist");
> > +     if (tmp) {
>
> Is there any reason why tmp is declared for whole function instead as
> variable used only in this block?
>

tmp is used several times in this function, but tmp2 could trivially be
placed within a tighter scope.

On Thu, Jan 20, 2011 at 5:36 PM, Maciej Pijanka <maciej.pijanka at gmail.com>
 wrote:

> > +             if (strspn(tmp2, "0123456789abcdef") != i) {
> > +                     msg_perr("Invalid chars in SPI command
> blacklist\n");
> > +                     free(tmp);
> > +                     return 1;
> > +             }
>

ABCDEF should probably be included in that set. Alternatively, maybe replace
the if statement with a loop that checks each character in tmp2 using
isxdigit(). The advantage with that approach is that you could tell the user
which character(s) are invalid.

-- 
David Hendricks (dhendrix)
Systems Software Engineer, Google Inc.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.flashrom.org/pipermail/flashrom/attachments/20110203/2ad3492f/attachment.html>