Re: [flashrom] [PATCH] Fix verification operation.

[Carl-Daniel Hailfinger]

Am 12.08.2013 02:00 schrieb Stefan Tauner:
> I broke this in r1702 where I enabled avoidance of the verification step if
> we did not modify anything in the erase/write step. The problem is that
> all_skipped is initialized to true and hence it would only ever verify if
> there have been changes noted in the erase/write step. This obviously
> breaks the verification operation (-v/--verify) because there we never
> enter the erase/write loop.
>
> The better alternative would be to enable (the implicit) verification
> in the write loop and not in cli_classic.c. This would require a bigger
> change due to the existance of dont_verify_it. Eventually this is the
> right thing to do but not so shortly before a release.
>
> Signed-off-by: Stefan Tauner

Acked-by: Carl-Daniel Hailfinger
> diff --git a/flashrom.c b/flashrom.c
> index db5b1c7..41e7bd8 100644
> --- a/flashrom.c
> +++ b/flashrom.c
> @@ -335,6 +335,9 @@ struct shutdown_func_data {
>   */
>  static int may_register_shutdown = 0;
>
> +/* Did we change something or was every erase/write skipped (if any)? */
> +static bool all_skipped = true;

This static variable will be a PITA for libflashrom. We should move it
into struct flashctx after the release.
> static int check_block_eraser(const struct flashctx *flash, int k, int log);
>
> /* Register a function to be executed on programmer shutdown.
> @@ -1309,7 +1312,6 @@ static int selfcheck_eraseblocks(const struct flashchip *chip)
>    return ret;
>  }
>
> -static bool all_skipped = true;
>  static int erase_and_write_block_helper(struct flashctx *flash,
>    				unsigned int start, unsigned int len,
>    				uint8_t *curcontents,

Regards,
Carl-Daniel

[PATCH] Fix verification operation.

[Stefan Tauner]

On Tue, 13 Aug 2013 00:28:32 +0200
Carl-Daniel Hailfinger  wrote:
> Am 12.08.2013 02:00 schrieb Stefan Tauner:
> > +/* Did we change something or was every erase/write skipped (if any)? */
> > +static bool all_skipped = true;
>
> This static variable will be a PITA for libflashrom. We should move it
> into struct flashctx after the release.

Or come up with something even better, definitely.

Thanks, r1707