Re: [Qemu-devel] [PATCH for 2.0] ide: Correct improper smart self test counter reset in ide core.

[Kevin Wolf]

Am 14.04.2014 um 14:10 hat Markus Armbruster geschrieben:
> Markus Armbruster <address@hidden> writes:
> 
> > Benoît Canet <address@hidden> writes:
> >
> >> The counter being reseted to zero make the array index negative.
> >> Reset it to 1.
> >>
> >> Signed-off-by: Benoit Canet <address@hidden>
> >> ---
> >>  hw/ide/core.c |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/hw/ide/core.c b/hw/ide/core.c
> >> index e1dfe54..c943a4d 100644
> >> --- a/hw/ide/core.c
> >> +++ b/hw/ide/core.c
> >> @@ -1602,7 +1602,7 @@ static bool cmd_smart(IDEState *s, uint8_t cmd)
> >>          case 2: /* extended self test */
> >>              s->smart_selftest_count++;
> >>              if (s->smart_selftest_count > 21) {
> >> -                s->smart_selftest_count = 0;
> >> +                s->smart_selftest_count = 1;
> >>              }
> >>              n = 2 + (s->smart_selftest_count - 1) * 24;
> >>              s->smart_selftest_data[n] = s->sector;
> >
> > Good catch.
> >
> > Commit message could use some love, though.  On every 21st SMART EXECUTE
> > OFFLINE:
> >
> > * We write before a dynamically allocated buffer
> >
> >   Your diff's context has one of the writes.
> >
> > * We forget SMART history
> >
> >   See the s->smart_selftest_count == 0 special cases in SMART READ DATA
> >   and SMART READ LOG.
> 
> Peter offered to improve the commit message on merge.  Even without
> that, it would be better to get this into 2.0 with a sub-optimal commit
> message than not to get it, so:
> 
> Reviewed-by: Markus Armbruster <address@hidden>

Should also be fixed in the stable branch of earlier releases. The bug
is present since SMART emulation was added in 2009.

Cc: address@hidden
Acked-by: Kevin Wolf <address@hidden>