Re: [flashrom] [PATCH] Fix and clean up cli_classic.c

[Stefan Tauner]

On Mon, 18 Jul 2011 23:31:43 +0200
Carl-Daniel Hailfinger  wrote:
> Am 18.07.2011 14:13 schrieb Stefan Tauner:
> > thanks to Florian Zumbiehl for reporting
> > this (http://paste.flashrom.org/view.php?id=707).
>
> Can you credit him in the changelog? Thanks!

sure
> > carldani: you wrote you did not fix the -l/-i ordering problem "because
> > the new layout code may have different requirements anyway".
> > the first step to fix this is to extract the needed file and region
> > names from the command line (to use them after the parsing loop has
> > finished). this is somewhat independent from whatever we do with the
> > arguments then. if you agree i would like to fix this now (i did not
> > look for what is needed exactly yet), if not you can just ack the patch.
>
> The flashrom-chromium tree has a workaround for that. The issue here is
> that there can be an unlimited number of -i parameters, so you have to
> either create an array layout_images[] or build a combined string which
> needs to be split up afterwards. I think one variant is done by the
> chromium tree. That said, my long-term hope is to allow only one -i
> parameter which is split like the programmer parameters with ",".
>
> It all boils down to whether we want to provide the best possible user
> experience at the cost of an ugly hack or if we want to avoid such a
> hack and move to the new layout commandline interface after 0.9.4. With
> r1373 and your bugfix on top (which should be committed now regardless
> of whether we hack around the -i limitation or not) the silent
> corruption is fixed.

i think multiple -i switches are very nice for the users and us.
creating a growing array by reallocing a new slot at each -i occurrence
does not sound so bad too me. parsing a long -i string OTOH isn't so
nice, although it is matching current -p behavior better.
> Side note: Should we check for each flashrom parameter (except -V) if it
> was specified more than once?

probably. although it is an unnecessary limitation in the case it is
clear to do anyway (e.g. -ww is not really a problem). OTOH it
indicates the use either does not know what he is doing or he does but
does not care much. :)
> > Signed-off-by: Stefan Tauner
>
> Acked-by: Carl-Daniel Hailfinger

thanks, committed in r1374