| Message ID | 1397336390-24664-1-git-send-email-benoit.canet@irqsave.net |
|---|---|
| State | New |
| Headers | show |
Benoît Canet <benoit.canet@irqsave.net> writes: > The counter being reseted to zero make the array index negative. > Reset it to 1. > > Signed-off-by: Benoit Canet <benoit@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.
Markus Armbruster <armbru@redhat.com> writes: > Benoît Canet <benoit.canet@irqsave.net> writes: > >> The counter being reseted to zero make the array index negative. >> Reset it to 1. >> >> Signed-off-by: Benoit Canet <benoit@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 <armbru@redhat.com>
Am 14.04.2014 um 14:10 hat Markus Armbruster geschrieben: > Markus Armbruster <armbru@redhat.com> writes: > > > Benoît Canet <benoit.canet@irqsave.net> writes: > > > >> The counter being reseted to zero make the array index negative. > >> Reset it to 1. > >> > >> Signed-off-by: Benoit Canet <benoit@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 <armbru@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-stable@nongnu.org Acked-by: Kevin Wolf <kwolf@redhat.com>
On 14 April 2014 13:31, Kevin Wolf <kwolf@redhat.com> wrote: > Am 14.04.2014 um 14:10 hat Markus Armbruster geschrieben: >> Markus Armbruster <armbru@redhat.com> writes: >> >> > Benoît Canet <benoit.canet@irqsave.net> writes: >> > >> >> The counter being reseted to zero make the array index negative. >> >> Reset it to 1. >> >> >> >> Signed-off-by: Benoit Canet <benoit@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 <armbru@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-stable@nongnu.org > Acked-by: Kevin Wolf <kwolf@redhat.com> Applied to master with a tweaked commit message and the various r-b/cc/etc tags. thanks -- PMM
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;
The counter being reseted to zero make the array index negative. Reset it to 1. Signed-off-by: Benoit Canet <benoit@irqsave.net> --- hw/ide/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)