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

[Markus Armbruster]

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>