Re: [flashrom] [PATCH 1/5] Add opaque programmer registration infrastructure.

[Michael Karcher]

> > > +const struct opaque_programmer *opaque_programmer = &opaque_programmer_none;
> > >
> > > +int probe_opaque(struct flashchip *flash)
> > > +{
> > > if (!opaque_programmer->probe) {
> > > msg_perr("%s called, but this is not an opaque programmer. "
> > > 	 "Please report a bug at [e-mail]\n",
> > > 	 __func__);
> >
> > I don't really like the error message, I would prefer "%s called without
> > register_opaque" or something like that.
>
> What about "%s called before register_opaque_programmer" ?

Good idea, go for it.
> Index: flashrom-register_opaque_programmer/opaque.c
>
> --- flashrom-register_opaque_programmer/opaque.c	(Revision 0)
> +++ flashrom-register_opaque_programmer/opaque.c	(Revision 0)

[...]
> +#include <strings.h>

Huh?! What's that strange BSD compatibility header doing here?
> +#include <string.h>
> +#include "flash.h"
> +#include "flashchips.h"

[...]

[in flashchips.c]
> @@ -9005,6 +9027,7 @@
>    	.probe		= probe_spi_rdid,
>    	.write		= NULL,
>    },
> {
>   .vendor		= "Generic",
>   .name		= "unknown SPI chip (REMS)",

This extra line has already be claimed by Stefan Tauner - it's not yours
anymore! ;)
> Signed-off-by: Carl-Daniel Hailfinger

If you remove the <strings.h> include or provide a good reason for it, this is

Acked-by: Michael Karcher

Regards,
  Michael Karcher