Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
Am 04.02.2011 13:30, schrieb Anthony Liguori: On 02/02/2011 01:28 PM, Stefan Weil wrote: Hello, these are some patches which I found on my stack of open patches. All of them should go into 0.14, and at least some of them could also be applied to 0.13. This need the following Acks: [PATCH] hw/fmopl: Fix buffer access out-of-bounds errors (http://patchwork.ozlabs.org/patch/79054/) malc Malc wrote that the patch looks correct, but it is still uncommitted. Regards, Anthony Liguori [PATCH] Fix trivial endianness bugs (http://patchwork.ozlabs.org/patch/80223/) [PATCH] HACKING: Update status of format checking (http://lists.nongnu.org/archive/html/qemu-devel/2011-01/msg02476.html) [PATCH] mingw32: Fix definitions for PRId64, PRIx64, PRIu64, PRIo64 (http://patchwork.ozlabs.org/patch/74276/) These three patches are still missing in QEMU master. I don't think that any of the patches is still needed in 0.14. So in total four of these patches are still waiting for a committer (or a comment why they won't be committed). Thanks to the committers of the other patches. Regards, Stefan Weil
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
Am 04.02.2011 16:27, schrieb Markus Armbruster: Anthony Liguorianth...@codemonkey.ws writes: On 02/02/2011 01:28 PM, Stefan Weil wrote: [...] [PATCH 1/3] tests: Fix two memory leaks (http://patchwork.ozlabs.org/patch/79945/) [PATCH 2/3] check-qdict: Fix possible crash (http://patchwork.ozlabs.org/patch/79946/) Luiz I wouldn't bother with the second one for 0.14. Yes, we're reading lines from a file with %s, but it's a fixed file with known contents, no long lines, and we're reading it in a test program only developers ever use. As to the first one, Luiz has never touched that file. Neither have I, and it's not obvious to me why it should go into 0.14. [...] Ack for qemu master then?
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
Stefan Weil w...@mail.berlios.de writes: Am 04.02.2011 16:27, schrieb Markus Armbruster: Anthony Liguorianth...@codemonkey.ws writes: On 02/02/2011 01:28 PM, Stefan Weil wrote: [...] [PATCH 1/3] tests: Fix two memory leaks (http://patchwork.ozlabs.org/patch/79945/) [PATCH 2/3] check-qdict: Fix possible crash (http://patchwork.ozlabs.org/patch/79946/) Luiz I wouldn't bother with the second one for 0.14. Yes, we're reading lines from a file with %s, but it's a fixed file with known contents, no long lines, and we're reading it in a test program only developers ever use. As to the first one, Luiz has never touched that file. Neither have I, and it's not obvious to me why it should go into 0.14. [...] Ack for qemu master then? I wouldn't bother myself, but I won't object, either.
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
Am 07.02.2011 19:54, schrieb Luiz Capitulino: This wasn't denied, what Markus said is that this is test code and thus it isn't high priority for the (now released) 0.14 release. That's ok. Fixing the code in master is fine. Thanks, Stefan
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
On Fri, 04 Feb 2011 18:36:39 +0100 Stefan Weil w...@mail.berlios.de wrote: Am 04.02.2011 18:21, schrieb Anthony Liguori: On 02/04/2011 11:18 AM, Stefan Weil wrote: Am 04.02.2011 16:27, schrieb Markus Armbruster: Anthony Liguori anth...@codemonkey.ws writes: On 02/02/2011 01:28 PM, Stefan Weil wrote: [...] [PATCH 1/3] tests: Fix two memory leaks (http://patchwork.ozlabs.org/patch/79945/) [PATCH 2/3] check-qdict: Fix possible crash (http://patchwork.ozlabs.org/patch/79946/) Luiz I wouldn't bother with the second one for 0.14. Yes, we're reading lines from a file with %s, but it's a fixed file with known contents, no long lines, and we're reading it in a test program only developers ever use. As to the first one, Luiz has never touched that file. Neither have I, and it's not obvious to me why it should go into 0.14. [...] Even if the current code does not result in a real bug at the moment, it should get fixed: * Using tools like cppcheck (or others) to find bugs is good, because it finds bugs which are important. Sorting out unimportant bugs from the results wastes time which could be invested better, and this waste of time lasts forever until the unimportant bug will be fixed. The sooner you fix it, the better it is. No, this is not a good use of time. I've said multiple times in the past, I'm not interested in implementing work arounds for false positives in static analysis tools. We have enough real problems to fix, we don't need to waste cycles on psuedo problems. Regards, Anthony Liguori Hi Anthony, please accept that even if you said something multiple times, other people might have a different point of view. QEMU is team work, isn't it? Both positives are correct, there was no false positive: Reading strings from external files into limited memory without limiting their length is bad. This wasn't denied, what Markus said is that this is test code and thus it isn't high priority for the (now released) 0.14 release. Even if it works with some input data, this kind of programming will be copied by novice programmers and used with data which is critical. OMG, are they copying code from qemu?! In the second case, it might be a philosophical question whether resources like memory or files should be released explicitly. I tend to say yes, other people say no because the OS will release them automatically when the program terminates. But there is no doubt that the tool which says there is a leak is right. Regards, Stefan Weil
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
On Fri, Feb 04, 2011 at 06:30:24AM -0600, Anthony Liguori wrote: [PATCH 1/3] pci: Fix memory leak (http://patchwork.ozlabs.org/patch/79996/) mst (looks like he did) Yes, ack.
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
On 02/04/2011 02:30 PM, Anthony Liguori wrote: This need the following Acks: [PATCH] linux-user: Fix possible realloc memory leak (http://patchwork.ozlabs.org/patch/79217/) Riku I thought I gave an OK for this already. Acked-By: Riku Voipio riku.voi...@iki.fi
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
On 02/02/2011 01:28 PM, Stefan Weil wrote: Hello, these are some patches which I found on my stack of open patches. All of them should go into 0.14, and at least some of them could also be applied to 0.13. This need the following Acks: [PATCH] hw/fmopl: Fix buffer access out-of-bounds errors (http://patchwork.ozlabs.org/patch/79054/) malc [PATCH] linux-user: Fix possible realloc memory leak (http://patchwork.ozlabs.org/patch/79217/) Riku [PATCH 1/3] tests: Fix two memory leaks (http://patchwork.ozlabs.org/patch/79945/) [PATCH 2/3] check-qdict: Fix possible crash (http://patchwork.ozlabs.org/patch/79946/) Luiz [PATCH 3/3] w64: Fix problem with missing sigset_t (http://patchwork.ozlabs.org/patch/79947/) We don't support w64... [PATCH 1/3] pci: Fix memory leak (http://patchwork.ozlabs.org/patch/79996/) mst (looks like he did) [PATCH 2/3] ppc405: Fix memory leak (http://patchwork.ozlabs.org/patch/79997/) [PATCH 3/3] s390: Fix memory leak (http://patchwork.ozlabs.org/patch/79998/) Alex for both of these. Regards, Anthony Liguori [PATCH] Fix trivial endianness bugs (http://patchwork.ozlabs.org/patch/80223/) [PATCH] HACKING: Update status of format checking (http://lists.nongnu.org/archive/html/qemu-devel/2011-01/msg02476.html) [PATCH] mingw32: Fix definitions for PRId64, PRIx64, PRIu64, PRIo64 (http://patchwork.ozlabs.org/patch/74276/) Regards, Stefan Weil
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
On 02/04/2011 01:30 PM, Anthony Liguori wrote: [PATCH 3/3] w64: Fix problem with missing sigset_t (http://patchwork.ozlabs.org/patch/79947/) We don't support w64... AFAIK mingw-w64 supports both 32-bit and 64-bit compilation, so the patch subject is misleading. Paolo
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
On 04.02.2011, at 13:30, Anthony Liguori anth...@codemonkey.ws wrote: On 02/02/2011 01:28 PM, Stefan Weil wrote: Hello, these are some patches which I found on my stack of open patches. All of them should go into 0.14, and at least some of them could also be applied to 0.13. This need the following Acks: [PATCH] hw/fmopl: Fix buffer access out-of-bounds errors (http://patchwork.ozlabs.org/patch/79054/) malc [PATCH] linux-user: Fix possible realloc memory leak (http://patchwork.ozlabs.org/patch/79217/) Riku [PATCH 1/3] tests: Fix two memory leaks (http://patchwork.ozlabs.org/patch/79945/) [PATCH 2/3] check-qdict: Fix possible crash (http://patchwork.ozlabs.org/patch/79946/) Luiz [PATCH 3/3] w64: Fix problem with missing sigset_t (http://patchwork.ozlabs.org/patch/79947/) We don't support w64... [PATCH 1/3] pci: Fix memory leak (http://patchwork.ozlabs.org/patch/79996/) mst (looks like he did) [PATCH 2/3] ppc405: Fix memory leak (http://patchwork.ozlabs.org/patch/79997/) [PATCH 3/3] s390: Fix memory leak (http://patchwork.ozlabs.org/patch/79998/) Alex for both of these. Acked-by: Alexander Graf ag...@suse.de for both :) Alex Regards, Anthony Liguori [PATCH] Fix trivial endianness bugs (http://patchwork.ozlabs.org/patch/80223/) [PATCH] HACKING: Update status of format checking (http://lists.nongnu.org/archive/html/qemu-devel/2011-01/msg02476.html) [PATCH] mingw32: Fix definitions for PRId64, PRIx64, PRIu64, PRIo64 (http://patchwork.ozlabs.org/patch/74276/) Regards, Stefan Weil
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
Anthony Liguori anth...@codemonkey.ws writes: On 02/02/2011 01:28 PM, Stefan Weil wrote: [...] [PATCH 1/3] tests: Fix two memory leaks (http://patchwork.ozlabs.org/patch/79945/) [PATCH 2/3] check-qdict: Fix possible crash (http://patchwork.ozlabs.org/patch/79946/) Luiz I wouldn't bother with the second one for 0.14. Yes, we're reading lines from a file with %s, but it's a fixed file with known contents, no long lines, and we're reading it in a test program only developers ever use. As to the first one, Luiz has never touched that file. Neither have I, and it's not obvious to me why it should go into 0.14. [...]
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
On 02/04/2011 09:27 AM, Markus Armbruster wrote: Anthony Liguorianth...@codemonkey.ws writes: On 02/02/2011 01:28 PM, Stefan Weil wrote: [...] [PATCH 1/3] tests: Fix two memory leaks (http://patchwork.ozlabs.org/patch/79945/) [PATCH 2/3] check-qdict: Fix possible crash (http://patchwork.ozlabs.org/patch/79946/) Luiz I wouldn't bother with the second one for 0.14. Yes, we're reading lines from a file with %s, but it's a fixed file with known contents, no long lines, and we're reading it in a test program only developers ever use. As to the first one, Luiz has never touched that file. Neither have I, and it's not obvious to me why it should go into 0.14. Yeah, I just meant the qdict one. Regards, Anthony Liguori [...]
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
On Fri, 4 Feb 2011, Anthony Liguori wrote: On 02/02/2011 01:28 PM, Stefan Weil wrote: Hello, these are some patches which I found on my stack of open patches. All of them should go into 0.14, and at least some of them could also be applied to 0.13. This need the following Acks: [PATCH] hw/fmopl: Fix buffer access out-of-bounds errors (http://patchwork.ozlabs.org/patch/79054/) malc The patch looks correct. [..snip..] -- mailto:av1...@comtv.ru
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
On Fri, 04 Feb 2011 16:27:46 +0100 Markus Armbruster arm...@redhat.com wrote: Anthony Liguori anth...@codemonkey.ws writes: On 02/02/2011 01:28 PM, Stefan Weil wrote: [...] [PATCH 1/3] tests: Fix two memory leaks (http://patchwork.ozlabs.org/patch/79945/) [PATCH 2/3] check-qdict: Fix possible crash (http://patchwork.ozlabs.org/patch/79946/) Luiz I wouldn't bother with the second one for 0.14. Yes, we're reading lines from a file with %s, but it's a fixed file with known contents, no long lines, and we're reading it in a test program only developers ever use. Agreed. As to the first one, Luiz has never touched that file. Neither have I, and it's not obvious to me why it should go into 0.14. [...]
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
Am 04.02.2011 16:27, schrieb Markus Armbruster: Anthony Liguori anth...@codemonkey.ws writes: On 02/02/2011 01:28 PM, Stefan Weil wrote: [...] [PATCH 1/3] tests: Fix two memory leaks (http://patchwork.ozlabs.org/patch/79945/) [PATCH 2/3] check-qdict: Fix possible crash (http://patchwork.ozlabs.org/patch/79946/) Luiz I wouldn't bother with the second one for 0.14. Yes, we're reading lines from a file with %s, but it's a fixed file with known contents, no long lines, and we're reading it in a test program only developers ever use. As to the first one, Luiz has never touched that file. Neither have I, and it's not obvious to me why it should go into 0.14. [...] Even if the current code does not result in a real bug at the moment, it should get fixed: * Using tools like cppcheck (or others) to find bugs is good, because it finds bugs which are important. Sorting out unimportant bugs from the results wastes time which could be invested better, and this waste of time lasts forever until the unimportant bug will be fixed. The sooner you fix it, the better it is. * Code gets copied, and maybe the copy of code with some weakness can expose a real problem. Therefore I think that both patches should be applied at least to qemu master (as they are really simple patches, applying them to 0.14 would be cheap and good as well). Regards, Stefan Weil
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
On 02/04/2011 11:18 AM, Stefan Weil wrote: Am 04.02.2011 16:27, schrieb Markus Armbruster: Anthony Liguori anth...@codemonkey.ws writes: On 02/02/2011 01:28 PM, Stefan Weil wrote: [...] [PATCH 1/3] tests: Fix two memory leaks (http://patchwork.ozlabs.org/patch/79945/) [PATCH 2/3] check-qdict: Fix possible crash (http://patchwork.ozlabs.org/patch/79946/) Luiz I wouldn't bother with the second one for 0.14. Yes, we're reading lines from a file with %s, but it's a fixed file with known contents, no long lines, and we're reading it in a test program only developers ever use. As to the first one, Luiz has never touched that file. Neither have I, and it's not obvious to me why it should go into 0.14. [...] Even if the current code does not result in a real bug at the moment, it should get fixed: * Using tools like cppcheck (or others) to find bugs is good, because it finds bugs which are important. Sorting out unimportant bugs from the results wastes time which could be invested better, and this waste of time lasts forever until the unimportant bug will be fixed. The sooner you fix it, the better it is. No, this is not a good use of time. I've said multiple times in the past, I'm not interested in implementing work arounds for false positives in static analysis tools. We have enough real problems to fix, we don't need to waste cycles on psuedo problems. Regards, Anthony Liguori * Code gets copied, and maybe the copy of code with some weakness can expose a real problem. Therefore I think that both patches should be applied at least to qemu master (as they are really simple patches, applying them to 0.14 would be cheap and good as well). Regards, Stefan Weil
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
Am 04.02.2011 18:21, schrieb Anthony Liguori: On 02/04/2011 11:18 AM, Stefan Weil wrote: Am 04.02.2011 16:27, schrieb Markus Armbruster: Anthony Liguori anth...@codemonkey.ws writes: On 02/02/2011 01:28 PM, Stefan Weil wrote: [...] [PATCH 1/3] tests: Fix two memory leaks (http://patchwork.ozlabs.org/patch/79945/) [PATCH 2/3] check-qdict: Fix possible crash (http://patchwork.ozlabs.org/patch/79946/) Luiz I wouldn't bother with the second one for 0.14. Yes, we're reading lines from a file with %s, but it's a fixed file with known contents, no long lines, and we're reading it in a test program only developers ever use. As to the first one, Luiz has never touched that file. Neither have I, and it's not obvious to me why it should go into 0.14. [...] Even if the current code does not result in a real bug at the moment, it should get fixed: * Using tools like cppcheck (or others) to find bugs is good, because it finds bugs which are important. Sorting out unimportant bugs from the results wastes time which could be invested better, and this waste of time lasts forever until the unimportant bug will be fixed. The sooner you fix it, the better it is. No, this is not a good use of time. I've said multiple times in the past, I'm not interested in implementing work arounds for false positives in static analysis tools. We have enough real problems to fix, we don't need to waste cycles on psuedo problems. Regards, Anthony Liguori Hi Anthony, please accept that even if you said something multiple times, other people might have a different point of view. QEMU is team work, isn't it? Both positives are correct, there was no false positive: Reading strings from external files into limited memory without limiting their length is bad. Even if it works with some input data, this kind of programming will be copied by novice programmers and used with data which is critical. In the second case, it might be a philosophical question whether resources like memory or files should be released explicitly. I tend to say yes, other people say no because the OS will release them automatically when the program terminates. But there is no doubt that the tool which says there is a leak is right. Regards, Stefan Weil
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
On 02/04/2011 11:36 AM, Stefan Weil wrote: Hi Anthony, please accept that even if you said something multiple times, other people might have a different point of view. Yup, just making my point of view clear. QEMU is team work, isn't it? Both positives are correct, there was no false positive: Reading strings from external files into limited memory without limiting their length is bad. Even if it works with some input data, this kind of programming will be copied by novice programmers and used with data which is critical. This is why I dislike patches like this, because the discussion about whether it really is important or not ends up being a huge distraction. Regards, Anthony Liguori
Re: [Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
Also: From: Gleb Natapov g...@redhat.com Subject: [Qemu-devel] [PATCH] do not pass NULL to strdup. Date: Wed, 2 Feb 2011 17:34:34 +0200 Message-ID: 20110202153434.gp14...@redhat.com
[Qemu-devel] [PING 0.14] Missing patches (mostly fixes)
Hello, these are some patches which I found on my stack of open patches. All of them should go into 0.14, and at least some of them could also be applied to 0.13. [PATCH] hw/fmopl: Fix buffer access out-of-bounds errors (http://patchwork.ozlabs.org/patch/79054/) [PATCH] linux-user: Fix possible realloc memory leak (http://patchwork.ozlabs.org/patch/79217/) [PATCH 1/3] tests: Fix two memory leaks (http://patchwork.ozlabs.org/patch/79945/) [PATCH 2/3] check-qdict: Fix possible crash (http://patchwork.ozlabs.org/patch/79946/) [PATCH 3/3] w64: Fix problem with missing sigset_t (http://patchwork.ozlabs.org/patch/79947/) [PATCH 1/3] pci: Fix memory leak (http://patchwork.ozlabs.org/patch/79996/) [PATCH 2/3] ppc405: Fix memory leak (http://patchwork.ozlabs.org/patch/79997/) [PATCH 3/3] s390: Fix memory leak (http://patchwork.ozlabs.org/patch/79998/) [PATCH] Fix trivial endianness bugs (http://patchwork.ozlabs.org/patch/80223/) [PATCH] HACKING: Update status of format checking (http://lists.nongnu.org/archive/html/qemu-devel/2011-01/msg02476.html) [PATCH] mingw32: Fix definitions for PRId64, PRIx64, PRIu64, PRIo64 (http://patchwork.ozlabs.org/patch/74276/) Regards, Stefan Weil
