* [Qemu-devel] [PATCH] target-mips: fix call to memset in soft reset code
@ 2016-05-09 16:44 Aurelien Jarno
2016-05-09 17:53 ` Stefan Weil
2016-05-12 11:45 ` Leon Alrae
0 siblings, 2 replies; 11+ messages in thread
From: Aurelien Jarno @ 2016-05-09 16:44 UTC (permalink / raw)
To: qemu-devel; +Cc: Aurelien Jarno, Stefan Weil, Leon Alrae
Recent versions of GCC report the following error when compiling
target-mips/helper.c:
qemu/target-mips/helper.c:542:9: warning: ‘memset’ used with length
equal to number of elements without multiplication by element size
[-Wmemset-elt-size]
This is indeed correct and due to a wrong usage of sizeof(). Fix that.
Cc: Stefan Weil <sw@weilnetz.de>
Cc: Leon Alrae <leon.alrae@imgtec.com>
LP: https://bugs.launchpad.net/qemu/+bug/1577841
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
target-mips/helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target-mips/helper.c b/target-mips/helper.c
index 1004ede..cfea177 100644
--- a/target-mips/helper.c
+++ b/target-mips/helper.c
@@ -539,7 +539,7 @@ void mips_cpu_do_interrupt(CPUState *cs)
break;
case EXCP_SRESET:
env->CP0_Status |= (1 << CP0St_SR);
- memset(env->CP0_WatchLo, 0, sizeof(*env->CP0_WatchLo));
+ memset(env->CP0_WatchLo, 0, sizeof(env->CP0_WatchLo));
goto set_error_EPC;
case EXCP_NMI:
env->CP0_Status |= (1 << CP0St_NMI);
--
2.8.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target-mips: fix call to memset in soft reset code
2016-05-09 16:44 [Qemu-devel] [PATCH] target-mips: fix call to memset in soft reset code Aurelien Jarno
@ 2016-05-09 17:53 ` Stefan Weil
2016-05-09 17:55 ` Peter Maydell
2016-05-12 11:45 ` Leon Alrae
1 sibling, 1 reply; 11+ messages in thread
From: Stefan Weil @ 2016-05-09 17:53 UTC (permalink / raw)
To: Aurelien Jarno, qemu-devel; +Cc: Leon Alrae, Peter Maydell
Am 09.05.2016 um 18:44 schrieb Aurelien Jarno:
> Recent versions of GCC report the following error when compiling
> target-mips/helper.c:
>
> qemu/target-mips/helper.c:542:9: warning: ‘memset’ used with length
> equal to number of elements without multiplication by element size
> [-Wmemset-elt-size]
>
> This is indeed correct and due to a wrong usage of sizeof(). Fix that.
>
> Cc: Stefan Weil <sw@weilnetz.de>
> Cc: Leon Alrae <leon.alrae@imgtec.com>
> LP: https://bugs.launchpad.net/qemu/+bug/1577841
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
> target-mips/helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-mips/helper.c b/target-mips/helper.c
> index 1004ede..cfea177 100644
> --- a/target-mips/helper.c
> +++ b/target-mips/helper.c
> @@ -539,7 +539,7 @@ void mips_cpu_do_interrupt(CPUState *cs)
> break;
> case EXCP_SRESET:
> env->CP0_Status |= (1 << CP0St_SR);
> - memset(env->CP0_WatchLo, 0, sizeof(*env->CP0_WatchLo));
> + memset(env->CP0_WatchLo, 0, sizeof(env->CP0_WatchLo));
> goto set_error_EPC;
> case EXCP_NMI:
> env->CP0_Status |= (1 << CP0St_NMI);
>
Reviewed-by: Stefan Weil <sw@weilnetz.de>
I suggest to apply this patch to 2.6, if this is still possible:
* It is a very small modification which fixes a bug.
* It fixes a new build error with recent gcc versions.
The first reason alone would not justify it for 2.6 as this
is a rather old bug.
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target-mips: fix call to memset in soft reset code
2016-05-09 17:53 ` Stefan Weil
@ 2016-05-09 17:55 ` Peter Maydell
2016-05-09 17:57 ` Peter Maydell
0 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2016-05-09 17:55 UTC (permalink / raw)
To: Stefan Weil; +Cc: Aurelien Jarno, QEMU Developers, Leon Alrae
On 9 May 2016 at 18:53, Stefan Weil <sw@weilnetz.de> wrote:
> I suggest to apply this patch to 2.6, if this is still possible
It is not; sorry.
thanks
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target-mips: fix call to memset in soft reset code
2016-05-09 17:55 ` Peter Maydell
@ 2016-05-09 17:57 ` Peter Maydell
2016-05-11 19:28 ` Christian Borntraeger
0 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2016-05-09 17:57 UTC (permalink / raw)
To: Stefan Weil; +Cc: Aurelien Jarno, QEMU Developers, Leon Alrae
On 9 May 2016 at 18:55, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 9 May 2016 at 18:53, Stefan Weil <sw@weilnetz.de> wrote:
>> I suggest to apply this patch to 2.6, if this is still possible
>
> It is not; sorry.
Note that it's only an error if you're building with -Werror, and
releases don't default to -Werror, so users using released QEMU 2.6
shouldn't hit this even with the newer gcc. Developers developing
on trunk shouldn't be unduly inconvenienced if the commit fixing it
is post-2.6-release rather than the actual release.
thanks
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target-mips: fix call to memset in soft reset code
2016-05-09 17:57 ` Peter Maydell
@ 2016-05-11 19:28 ` Christian Borntraeger
2016-05-11 22:41 ` Peter Maydell
0 siblings, 1 reply; 11+ messages in thread
From: Christian Borntraeger @ 2016-05-11 19:28 UTC (permalink / raw)
To: Peter Maydell, Stefan Weil; +Cc: Leon Alrae, QEMU Developers, Aurelien Jarno
On 05/09/2016 07:57 PM, Peter Maydell wrote:
> On 9 May 2016 at 18:55, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 9 May 2016 at 18:53, Stefan Weil <sw@weilnetz.de> wrote:
>>> I suggest to apply this patch to 2.6, if this is still possible
>>
>> It is not; sorry.
I think we have delayed 2.6 already far too long (so please release)
but
> Note that it's only an error if you're building with -Werror, and
> releases don't default to -Werror, so users using released QEMU 2.6
> shouldn't hit this even with the newer gcc. Developers developing
> on trunk shouldn't be unduly inconvenienced if the commit fixing it
> is post-2.6-release rather than the actual release.
to me it looks like that this is not a compile time error, instead we
really do not memset a variable that we are supposed to memset.
the only reason to not consider it for 2.6 is that its is really old
and it did not seem to cause harm.
Christian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target-mips: fix call to memset in soft reset code
2016-05-11 19:28 ` Christian Borntraeger
@ 2016-05-11 22:41 ` Peter Maydell
2016-05-12 7:40 ` Christian Borntraeger
0 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2016-05-11 22:41 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Stefan Weil, Leon Alrae, QEMU Developers, Aurelien Jarno
On 11 May 2016 at 20:28, Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> On 05/09/2016 07:57 PM, Peter Maydell wrote:
>> On 9 May 2016 at 18:55, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 9 May 2016 at 18:53, Stefan Weil <sw@weilnetz.de> wrote:
>>>> I suggest to apply this patch to 2.6, if this is still possible
>>>
>>> It is not; sorry.
>
> I think we have delayed 2.6 already far too long (so please release)
> but
Already done :-)
>> Note that it's only an error if you're building with -Werror, and
>> releases don't default to -Werror, so users using released QEMU 2.6
>> shouldn't hit this even with the newer gcc. Developers developing
>> on trunk shouldn't be unduly inconvenienced if the commit fixing it
>> is post-2.6-release rather than the actual release.
>
> to me it looks like that this is not a compile time error, instead we
> really do not memset a variable that we are supposed to memset.
> the only reason to not consider it for 2.6 is that its is really old
> and it did not seem to cause harm.
Yes, it is both a code bug and a compile error, but the former
has been present for many releases so is not a regression, and
the latter is only an error if you're building with -Werror.
So in my view it's the kind of bug we'd certainly fix at about
rc3 or so, but not a bug which is "release critical showstopper".
thanks
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target-mips: fix call to memset in soft reset code
2016-05-11 22:41 ` Peter Maydell
@ 2016-05-12 7:40 ` Christian Borntraeger
2016-05-12 8:04 ` Peter Maydell
2016-05-12 8:06 ` Cornelia Huck
0 siblings, 2 replies; 11+ messages in thread
From: Christian Borntraeger @ 2016-05-12 7:40 UTC (permalink / raw)
To: Peter Maydell; +Cc: Stefan Weil, Leon Alrae, QEMU Developers, Aurelien Jarno
On 05/12/2016 12:41 AM, Peter Maydell wrote:
> On 11 May 2016 at 20:28, Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>> On 05/09/2016 07:57 PM, Peter Maydell wrote:
>>> On 9 May 2016 at 18:55, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>> On 9 May 2016 at 18:53, Stefan Weil <sw@weilnetz.de> wrote:
>>>>> I suggest to apply this patch to 2.6, if this is still possible
>>>>
>>>> It is not; sorry.
>>
>> I think we have delayed 2.6 already far too long (so please release)
>> but
>
> Already done :-)
>
>>> Note that it's only an error if you're building with -Werror, and
>>> releases don't default to -Werror, so users using released QEMU 2.6
>>> shouldn't hit this even with the newer gcc. Developers developing
>>> on trunk shouldn't be unduly inconvenienced if the commit fixing it
>>> is post-2.6-release rather than the actual release.
>>
>> to me it looks like that this is not a compile time error, instead we
>> really do not memset a variable that we are supposed to memset.
>> the only reason to not consider it for 2.6 is that its is really old
>> and it did not seem to cause harm.
>
> Yes, it is both a code bug and a compile error, but the former
> has been present for many releases so is not a regression, and
> the latter is only an error if you're building with -Werror.
> So in my view it's the kind of bug we'd certainly fix at about
> rc3 or so, but not a bug which is "release critical showstopper".
Maybe a topic for this years QEMU summit could be to talk about
release process and release criterias.
We could
a: allow more patches , e.g. I thing that this patch would be have
been taken in the Linux kernel a day before the release, see for
example what is applied 4 days before a release as network fixes:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4d8bbbff1227bbb27fdb02b6db17f080c06eedad
22 files changed, 258 insertions, 86 deletions
b: as we are strict and only apply hand selected patches, regressions are
very unlikely, so we could release sooner. For example the CVE fixes could
have just been taken and rc5 being released as final. (which we did anyway
but 3 days later)
c: we consider everything fine and keep the process
d: better ideas
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target-mips: fix call to memset in soft reset code
2016-05-12 7:40 ` Christian Borntraeger
@ 2016-05-12 8:04 ` Peter Maydell
2016-05-12 8:06 ` Cornelia Huck
1 sibling, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2016-05-12 8:04 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Stefan Weil, Leon Alrae, QEMU Developers, Aurelien Jarno
On 12 May 2016 at 08:40, Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> Maybe a topic for this years QEMU summit could be to talk about
> release process and release criterias.
Yeah, I'm happy to talk about what we could do better with
releases (both on the mailing list and at the summit). A
couple of notes:
This time around we had to delay by at least a week because of
the timing of the CVEs -- we had to allow a reasonable time
before raising the embargo for distros to prepare fixes, and
the bugs came in pretty close to when we'd otherwise have done
our final rc for the release.
I think one significant difference between us and Linux is that
we have fewer people testing our rcs, so I worry that if we put
more stuff in then we are less likely to notice bugs in it
in time.
The intended purpose of the "few days gap then final release
is same as last rc" approach is to give a safety valve in
case the patches that went into the final rc had some
horrendous bug in them. In this case the last rc only had the
CVE fixes so was pretty safe, but in previous releases we've
often had a few more patches than that in final-rc. I don't
think two extra days before reopening the tree is a very
big cost.
thanks
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target-mips: fix call to memset in soft reset code
2016-05-12 7:40 ` Christian Borntraeger
2016-05-12 8:04 ` Peter Maydell
@ 2016-05-12 8:06 ` Cornelia Huck
2016-05-12 8:16 ` Christian Borntraeger
1 sibling, 1 reply; 11+ messages in thread
From: Cornelia Huck @ 2016-05-12 8:06 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Peter Maydell, Stefan Weil, Leon Alrae, QEMU Developers, Aurelien Jarno
On Thu, 12 May 2016 09:40:21 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> Maybe a topic for this years QEMU summit could be to talk about
> release process and release criterias.
+1 to that.
> We could
> a: allow more patches , e.g. I thing that this patch would be have
> been taken in the Linux kernel a day before the release, see for
> example what is applied 4 days before a release as network fixes:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4d8bbbff1227bbb27fdb02b6db17f080c06eedad
> 22 files changed, 258 insertions, 86 deletions
Personally, I would probably go for something between applying this
patch and that networking pull :)
> b: as we are strict and only apply hand selected patches, regressions are
> very unlikely, so we could release sooner. For example the CVE fixes could
> have just been taken and rc5 being released as final. (which we did anyway
> but 3 days later)
>
> c: we consider everything fine and keep the process
>
> d: better ideas
One thing I've noticed is that softfreeze/early hardfreeze qemus often
seem more unstable than versions earlier in the development cycle -
probably because people panic and rush to get code in for the release.
I don't know if stricter rules/enforcement of what is supposed to go in
during softfreeze/hardfreeze would help here.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target-mips: fix call to memset in soft reset code
2016-05-12 8:06 ` Cornelia Huck
@ 2016-05-12 8:16 ` Christian Borntraeger
0 siblings, 0 replies; 11+ messages in thread
From: Christian Borntraeger @ 2016-05-12 8:16 UTC (permalink / raw)
To: Cornelia Huck
Cc: Peter Maydell, Stefan Weil, Leon Alrae, QEMU Developers, Aurelien Jarno
On 05/12/2016 10:06 AM, Cornelia Huck wrote:
> On Thu, 12 May 2016 09:40:21 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>
>> Maybe a topic for this years QEMU summit could be to talk about
>> release process and release criterias.
>
> +1 to that.
>
>> We could
>> a: allow more patches , e.g. I thing that this patch would be have
>> been taken in the Linux kernel a day before the release, see for
>> example what is applied 4 days before a release as network fixes:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4d8bbbff1227bbb27fdb02b6db17f080c06eedad
>> 22 files changed, 258 insertions, 86 deletions
>
> Personally, I would probably go for something between applying this
> patch and that networking pull :)
>
>> b: as we are strict and only apply hand selected patches, regressions are
>> very unlikely, so we could release sooner. For example the CVE fixes could
>> have just been taken and rc5 being released as final. (which we did anyway
>> but 3 days later)
>>
>> c: we consider everything fine and keep the process
>>
>> d: better ideas
>
> One thing I've noticed is that softfreeze/early hardfreeze qemus often
> seem more unstable than versions earlier in the development cycle -
> probably because people panic and rush to get code in for the release.
> I don't know if stricter rules/enforcement of what is supposed to go in
> during softfreeze/hardfreeze would help here.
Yes, there are some problems here. But I fully agree with Peter.
We really do need more infrastructure and people to
- maintain stable release much more often and for more than one release
(that would allow to be less strict before a release)
- have something like qemu-next
- have something like kbuild test robot (aka 0-Day)
- have releases more often (to reduce the pressure to rush in)
Christian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] target-mips: fix call to memset in soft reset code
2016-05-09 16:44 [Qemu-devel] [PATCH] target-mips: fix call to memset in soft reset code Aurelien Jarno
2016-05-09 17:53 ` Stefan Weil
@ 2016-05-12 11:45 ` Leon Alrae
1 sibling, 0 replies; 11+ messages in thread
From: Leon Alrae @ 2016-05-12 11:45 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel, Stefan Weil, qemu-stable
On Mon, May 09, 2016 at 06:44:00PM +0200, Aurelien Jarno wrote:
> Recent versions of GCC report the following error when compiling
> target-mips/helper.c:
>
> qemu/target-mips/helper.c:542:9: warning: ‘memset’ used with length
> equal to number of elements without multiplication by element size
> [-Wmemset-elt-size]
>
> This is indeed correct and due to a wrong usage of sizeof(). Fix that.
>
> Cc: Stefan Weil <sw@weilnetz.de>
> Cc: Leon Alrae <leon.alrae@imgtec.com>
> LP: https://bugs.launchpad.net/qemu/+bug/1577841
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
> target-mips/helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied to target-mips queue and added Cc: qemu-stable@nongnu.org.
Thanks,
Leon
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-05-12 11:45 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-09 16:44 [Qemu-devel] [PATCH] target-mips: fix call to memset in soft reset code Aurelien Jarno
2016-05-09 17:53 ` Stefan Weil
2016-05-09 17:55 ` Peter Maydell
2016-05-09 17:57 ` Peter Maydell
2016-05-11 19:28 ` Christian Borntraeger
2016-05-11 22:41 ` Peter Maydell
2016-05-12 7:40 ` Christian Borntraeger
2016-05-12 8:04 ` Peter Maydell
2016-05-12 8:06 ` Cornelia Huck
2016-05-12 8:16 ` Christian Borntraeger
2016-05-12 11:45 ` Leon Alrae
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.