On 14 April 2014 13:31, Kevin Wolf <kw...@redhat.com> wrote:
> Am 14.04.2014 um 14:10 hat Markus Armbruster geschrieben:
>> Markus Armbruster <arm...@redhat.com> writes:
>>
>> > Benoît Canet <benoit.ca...@irqsave.net> writes:
>> >
>> >> The counter being reseted to zero make the array index negative.
>> >> Reset it to 1.
>> >>
>> >> Signed-off-by: Benoit Canet <ben...@irqsave.net>
>> >> ---
>> >> 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 <arm...@redhat.com>
>
> Should also be fixed in the stable branch of earlier releases. The bug
> is present since SMART emulation was added in 2009.
>
> Cc: qemu-sta...@nongnu.org
> Acked-by: Kevin Wolf <kw...@redhat.com>
Applied to master with a tweaked commit message and the various
r-b/cc/etc tags.
thanks
-- PMM
