* [Qemu-devel] [PATCH V2] build: remove compile warning
@ 2013-06-07 10:02 Wenchao Xia
2013-06-07 11:35 ` Stefan Hajnoczi
2013-06-07 12:17 ` Markus Armbruster
0 siblings, 2 replies; 13+ messages in thread
From: Wenchao Xia @ 2013-06-07 10:02 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, aliguori, mlureau, alevy, stefanha, pbonzini, Wenchao Xia
This patch simply remove "variable may be used uninitialized" warning.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
V2: Address Stefan and Peter's comments, use 0 in send_msg() instead of
initialize mhHeader.
libcacard/vscclient.c | 3 +--
util/iov.c | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index ac23647..7fbf1da 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -641,7 +641,6 @@ main(
GIOChannel *channel_stdin;
char *qemu_host;
char *qemu_port;
- VSCMsgHeader mhHeader;
VCardEmulOptions *command_line_options = NULL;
@@ -750,7 +749,7 @@ main(
.magic = VSCARD_MAGIC,
.capabilities = {0}
};
- send_msg(VSC_Init, mhHeader.reader_id, &init, sizeof(init));
+ send_msg(VSC_Init, 0, &init, sizeof(init));
g_main_loop_run(loop);
g_main_loop_unref(loop);
diff --git a/util/iov.c b/util/iov.c
index cc6e837..b91cfb9 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -146,7 +146,7 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt,
{
ssize_t total = 0;
ssize_t ret;
- size_t orig_len, tail;
+ size_t orig_len = 0, tail;
unsigned niov;
while (bytes > 0) {
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH V2] build: remove compile warning
2013-06-07 10:02 [Qemu-devel] [PATCH V2] build: remove compile warning Wenchao Xia
@ 2013-06-07 11:35 ` Stefan Hajnoczi
2013-06-07 12:17 ` Markus Armbruster
1 sibling, 0 replies; 13+ messages in thread
From: Stefan Hajnoczi @ 2013-06-07 11:35 UTC (permalink / raw)
To: Wenchao Xia; +Cc: peter.maydell, aliguori, mlureau, qemu-devel, alevy, pbonzini
On Fri, Jun 07, 2013 at 06:02:09PM +0800, Wenchao Xia wrote:
> This patch simply remove "variable may be used uninitialized" warning.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
> V2: Address Stefan and Peter's comments, use 0 in send_msg() instead of
> initialize mhHeader.
>
> libcacard/vscclient.c | 3 +--
> util/iov.c | 2 +-
> 2 files changed, 2 insertions(+), 3 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH V2] build: remove compile warning
2013-06-07 10:02 [Qemu-devel] [PATCH V2] build: remove compile warning Wenchao Xia
2013-06-07 11:35 ` Stefan Hajnoczi
@ 2013-06-07 12:17 ` Markus Armbruster
2013-06-08 2:04 ` Wenchao Xia
2013-06-18 10:13 ` Paolo Bonzini
1 sibling, 2 replies; 13+ messages in thread
From: Markus Armbruster @ 2013-06-07 12:17 UTC (permalink / raw)
To: Wenchao Xia
Cc: peter.maydell, aliguori, qemu-devel, mlureau, stefanha, pbonzini,
alevy, Robert Relyea
Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
> This patch simply remove "variable may be used uninitialized" warning.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
> V2: Address Stefan and Peter's comments, use 0 in send_msg() instead of
> initialize mhHeader.
>
> libcacard/vscclient.c | 3 +--
> util/iov.c | 2 +-
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
> index ac23647..7fbf1da 100644
> --- a/libcacard/vscclient.c
> +++ b/libcacard/vscclient.c
> @@ -641,7 +641,6 @@ main(
> GIOChannel *channel_stdin;
> char *qemu_host;
> char *qemu_port;
> - VSCMsgHeader mhHeader;
>
> VCardEmulOptions *command_line_options = NULL;
>
> @@ -750,7 +749,7 @@ main(
> .magic = VSCARD_MAGIC,
> .capabilities = {0}
> };
> - send_msg(VSC_Init, mhHeader.reader_id, &init, sizeof(init));
> + send_msg(VSC_Init, 0, &init, sizeof(init));
>
> g_main_loop_run(loop);
> g_main_loop_unref(loop);
This one's actually a bit more than just a warning suppression. The
uninitialized value gets passed to send_msg(), which prints it if
verbose > 10.
Makes no sense to me. Comes from commit 2ac85b9; cc'ing its author for
advice.
> diff --git a/util/iov.c b/util/iov.c
> index cc6e837..b91cfb9 100644
> --- a/util/iov.c
> +++ b/util/iov.c
> @@ -146,7 +146,7 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt,
> {
> ssize_t total = 0;
> ssize_t ret;
> - size_t orig_len, tail;
> + size_t orig_len = 0, tail;
> unsigned niov;
>
> while (bytes > 0) {
Here are the uses of orig_len:
if (tail) {
/* second, fixup the last element, and remember the original
* length */
assert(niov < iov_cnt);
assert(iov[niov].iov_len > tail);
orig_len = iov[niov].iov_len;
iov[niov++].iov_len = tail;
}
ret = do_send_recv(sockfd, iov, niov, do_send);
/* Undo the changes above before checking for errors */
if (tail) {
iov[niov-1].iov_len = orig_len;
}
gcc is too stupid to understand the control flow. The initialization
shuts it up.
Personally, I dislike "shut up" initializations, because when you
mistakenly adds a new uninitialized use, you get the arbitrary "shut up"
value without warning.
Possible alternative:
if (tail) {
/* second, fixup the last element, and remember the original
* length */
assert(niov < iov_cnt);
assert(iov[niov].iov_len > tail);
orig_len = iov[niov].iov_len;
iov[niov++].iov_len = tail;
ret = do_send_recv(sockfd, iov, niov, do_send);
/* Undo the changes above before checking for errors */
iov[niov-1].iov_len = orig_len;
} else {
ret = do_send_recv(sockfd, iov, niov, do_send);
}
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH V2] build: remove compile warning
2013-06-07 12:17 ` Markus Armbruster
@ 2013-06-08 2:04 ` Wenchao Xia
2013-06-18 10:14 ` Paolo Bonzini
2013-06-18 10:13 ` Paolo Bonzini
1 sibling, 1 reply; 13+ messages in thread
From: Wenchao Xia @ 2013-06-08 2:04 UTC (permalink / raw)
To: Markus Armbruster
Cc: peter.maydell, aliguori, qemu-devel, mlureau, stefanha, pbonzini,
alevy, Robert Relyea
I insist to remove compile warning since I want gcc check my code with
strict rule.
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
>
>> This patch simply remove "variable may be used uninitialized" warning.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>> V2: Address Stefan and Peter's comments, use 0 in send_msg() instead of
>> initialize mhHeader.
>>
>> libcacard/vscclient.c | 3 +--
>> util/iov.c | 2 +-
>> 2 files changed, 2 insertions(+), 3 deletions(-)
>>
>
>> diff --git a/util/iov.c b/util/iov.c
>> index cc6e837..b91cfb9 100644
>> --- a/util/iov.c
>> +++ b/util/iov.c
>> @@ -146,7 +146,7 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt,
>> {
>> ssize_t total = 0;
>> ssize_t ret;
>> - size_t orig_len, tail;
>> + size_t orig_len = 0, tail;
>> unsigned niov;
>>
>> while (bytes > 0) {
>
> Here are the uses of orig_len:
>
> if (tail) {
> /* second, fixup the last element, and remember the original
> * length */
> assert(niov < iov_cnt);
> assert(iov[niov].iov_len > tail);
> orig_len = iov[niov].iov_len;
> iov[niov++].iov_len = tail;
> }
>
> ret = do_send_recv(sockfd, iov, niov, do_send);
>
> /* Undo the changes above before checking for errors */
> if (tail) {
> iov[niov-1].iov_len = orig_len;
> }
>
>
> gcc is too stupid to understand the control flow. The initialization
> shuts it up.
>
> Personally, I dislike "shut up" initializations, because when you
> mistakenly adds a new uninitialized use, you get the arbitrary "shut up"
> value without warning.
>
> Possible alternative:
>
> if (tail) {
> /* second, fixup the last element, and remember the original
> * length */
> assert(niov < iov_cnt);
> assert(iov[niov].iov_len > tail);
> orig_len = iov[niov].iov_len;
> iov[niov++].iov_len = tail;
> ret = do_send_recv(sockfd, iov, niov, do_send);
> /* Undo the changes above before checking for errors */
> iov[niov-1].iov_len = orig_len;
> } else {
> ret = do_send_recv(sockfd, iov, niov, do_send);
> }
>
OK to work, but duplicated a line. I think it is not bad to give
default value as zero, even there will be new use later.
--
Best Regards
Wenchao Xia
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH V2] build: remove compile warning
2013-06-07 12:17 ` Markus Armbruster
2013-06-08 2:04 ` Wenchao Xia
@ 2013-06-18 10:13 ` Paolo Bonzini
2013-06-19 6:27 ` Wenchao Xia
2013-06-22 10:03 ` Stefan Weil
1 sibling, 2 replies; 13+ messages in thread
From: Paolo Bonzini @ 2013-06-18 10:13 UTC (permalink / raw)
To: Markus Armbruster
Cc: peter.maydell, aliguori, qemu-devel, Robert Relyea, alevy,
stefanha, mlureau, Wenchao Xia
Il 07/06/2013 14:17, Markus Armbruster ha scritto:
>> diff --git a/util/iov.c b/util/iov.c
>> index cc6e837..b91cfb9 100644
>> --- a/util/iov.c
>> +++ b/util/iov.c
>> @@ -146,7 +146,7 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt,
>> {
>> ssize_t total = 0;
>> ssize_t ret;
>> - size_t orig_len, tail;
>> + size_t orig_len = 0, tail;
>> unsigned niov;
>>
>> while (bytes > 0) {
>
> Here are the uses of orig_len:
>
> if (tail) {
> /* second, fixup the last element, and remember the original
> * length */
> assert(niov < iov_cnt);
> assert(iov[niov].iov_len > tail);
> orig_len = iov[niov].iov_len;
> iov[niov++].iov_len = tail;
> }
>
> ret = do_send_recv(sockfd, iov, niov, do_send);
>
> /* Undo the changes above before checking for errors */
> if (tail) {
> iov[niov-1].iov_len = orig_len;
> }
>
>
> gcc is too stupid to understand the control flow. The initialization
> shuts it up.
Looks like most people's GCC is not that stupid, or I would have broken
build for everyone, right?
Paolo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH V2] build: remove compile warning
2013-06-08 2:04 ` Wenchao Xia
@ 2013-06-18 10:14 ` Paolo Bonzini
0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2013-06-18 10:14 UTC (permalink / raw)
To: Wenchao Xia
Cc: peter.maydell, aliguori, Markus Armbruster, qemu-devel, alevy,
stefanha, mlureau, Robert Relyea
Il 08/06/2013 04:04, Wenchao Xia ha scritto:
> I insist to remove compile warning since I want gcc check my code with
> strict rule.
What is your version of GCC?
Paolo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH V2] build: remove compile warning
2013-06-18 10:13 ` Paolo Bonzini
@ 2013-06-19 6:27 ` Wenchao Xia
2013-06-19 9:33 ` Paolo Bonzini
2013-06-22 10:03 ` Stefan Weil
1 sibling, 1 reply; 13+ messages in thread
From: Wenchao Xia @ 2013-06-19 6:27 UTC (permalink / raw)
To: Paolo Bonzini
Cc: peter.maydell, aliguori, Markus Armbruster, qemu-devel, alevy,
stefanha, mlureau, Robert Relyea
于 2013-6-18 18:13, Paolo Bonzini 写道:
> Il 07/06/2013 14:17, Markus Armbruster ha scritto:
>>> diff --git a/util/iov.c b/util/iov.c
>>> index cc6e837..b91cfb9 100644
>>> --- a/util/iov.c
>>> +++ b/util/iov.c
>>> @@ -146,7 +146,7 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt,
>>> {
>>> ssize_t total = 0;
>>> ssize_t ret;
>>> - size_t orig_len, tail;
>>> + size_t orig_len = 0, tail;
>>> unsigned niov;
>>>
>>> while (bytes > 0) {
>>
>> Here are the uses of orig_len:
>>
>> if (tail) {
>> /* second, fixup the last element, and remember the original
>> * length */
>> assert(niov < iov_cnt);
>> assert(iov[niov].iov_len > tail);
>> orig_len = iov[niov].iov_len;
>> iov[niov++].iov_len = tail;
>> }
>>
>> ret = do_send_recv(sockfd, iov, niov, do_send);
>>
>> /* Undo the changes above before checking for errors */
>> if (tail) {
>> iov[niov-1].iov_len = orig_len;
>> }
>>
>>
>> gcc is too stupid to understand the control flow. The initialization
>> shuts it up.
>
> Looks like most people's GCC is not that stupid, or I would have broken
> build for everyone, right?
>
> Paolo
>
my gcc version:
[xiawenc@RH63Wenchao ~]$ gcc -v
Using built-in specs.
Target: x86_64-redhat-linux
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man
--infodir=/usr/share/info
--with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-bootstrap
--enable-shared --enable-threads=posix --enable-checking=release
--with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions
--enable-gnu-unique-object
--enable-languages=c,c++,objc,obj-c++,java,fortran,ada
--enable-java-awt=gtk --disable-dssi
--with-java-home=/usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre
--enable-libgcj-multifile --enable-java-maintainer-mode
--with-ecj-jar=/usr/share/java/eclipse-ecj.jar
--disable-libjava-multilib --with-ppl --with-cloog --with-tune=generic
--with-arch_32=i686 --build=x86_64-redhat-linux
Thread model: posix
gcc version 4.4.6 20120305 (Red Hat 4.4.6-4) (GCC)
By default configure, it seems qemu didn't set -Werror to break build.
--
Best Regards
Wenchao Xia
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH V2] build: remove compile warning
2013-06-19 6:27 ` Wenchao Xia
@ 2013-06-19 9:33 ` Paolo Bonzini
0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2013-06-19 9:33 UTC (permalink / raw)
To: Wenchao Xia
Cc: peter.maydell, aliguori, Markus Armbruster, qemu-devel, alevy,
stefanha, mlureau, Robert Relyea
Il 19/06/2013 08:27, Wenchao Xia ha scritto:
>>> gcc is too stupid to understand the control flow. The initialization
>>> shuts it up.
>>
>> Looks like most people's GCC is not that stupid, or I would have broken
>> build for everyone, right?
>
> my gcc version:
> [xiawenc@RH63Wenchao ~]$ gcc -v
> Using built-in specs.
> Target: x86_64-redhat-linux
> Configured with: ../configure --prefix=/usr --mandir=/usr/share/man
> --infodir=/usr/share/info
> --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-bootstrap
> --enable-shared --enable-threads=posix --enable-checking=release
> --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions
> --enable-gnu-unique-object
> --enable-languages=c,c++,objc,obj-c++,java,fortran,ada
> --enable-java-awt=gtk --disable-dssi
> --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre
> --enable-libgcj-multifile --enable-java-maintainer-mode
> --with-ecj-jar=/usr/share/java/eclipse-ecj.jar
> --disable-libjava-multilib --with-ppl --with-cloog --with-tune=generic
> --with-arch_32=i686 --build=x86_64-redhat-linux
> Thread model: posix
> gcc version 4.4.6 20120305 (Red Hat 4.4.6-4) (GCC)
>
>
> By default configure, it seems qemu didn't set -Werror to break build.
It sets -Werror here:
gcc (GCC) 4.7.2 20121109 (Red Hat 4.7.2-8)
So configure is doing the right thing for both old and new-enough
compilers, I don't think there's a reason to change the code.
Paolo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH V2] build: remove compile warning
2013-06-18 10:13 ` Paolo Bonzini
2013-06-19 6:27 ` Wenchao Xia
@ 2013-06-22 10:03 ` Stefan Weil
2013-06-24 14:44 ` Paolo Bonzini
1 sibling, 1 reply; 13+ messages in thread
From: Stefan Weil @ 2013-06-22 10:03 UTC (permalink / raw)
To: Paolo Bonzini
Cc: peter.maydell, aliguori, Wenchao Xia, Markus Armbruster,
qemu-devel, alevy, stefanha, mlureau, Robert Relyea
Am 18.06.2013 12:13, schrieb Paolo Bonzini:
> Il 07/06/2013 14:17, Markus Armbruster ha scritto:
>>> diff --git a/util/iov.c b/util/iov.c
>>> index cc6e837..b91cfb9 100644
>>> --- a/util/iov.c
>>> +++ b/util/iov.c
>>> @@ -146,7 +146,7 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt,
>>> {
>>> ssize_t total = 0;
>>> ssize_t ret;
>>> - size_t orig_len, tail;
>>> + size_t orig_len = 0, tail;
>>> unsigned niov;
>>>
>>> while (bytes > 0) {
>> Here are the uses of orig_len:
>>
>> if (tail) {
>> /* second, fixup the last element, and remember the original
>> * length */
>> assert(niov < iov_cnt);
>> assert(iov[niov].iov_len > tail);
>> orig_len = iov[niov].iov_len;
>> iov[niov++].iov_len = tail;
>> }
>>
>> ret = do_send_recv(sockfd, iov, niov, do_send);
>>
>> /* Undo the changes above before checking for errors */
>> if (tail) {
>> iov[niov-1].iov_len = orig_len;
>> }
>>
>>
>> gcc is too stupid to understand the control flow. The initialization
>> shuts it up.
> Looks like most people's GCC is not that stupid, or I would have broken
> build for everyone, right?
>
> Paolo
Hi Paolo,
I get this warning, too, when I run a normal cross compilation with
MinGW-w64:
util/iov.c:190:33: warning: ‘orig_len’ may be used uninitialized in this
function [-Wuninitialized]
My build environment:
Debian wheezy with packages gcc-mingw-w64-i686, gcc-mingw-w64-x86-64
(4.6.3-14+8).
A complete build results in 5 warnings. Here are the other 4 of them:
hw/arm/spitz.c:280:0: warning: "MOD_SHIFT" redefined [enabled by default]
hw/ppc/spapr.c:673:26: warning: cast from pointer to integer of
different size [-Wpointer-to-int-cast]
hw/ppc/spapr_hcall.c:188:1: warning: control reaches end of non-void
function [-Wreturn-type]
hw/ppc/spapr_pci.c:454:1: warning: control reaches end of non-void
function [-Wreturn-type]
I already sent a patch for the MOD_SHIFT issue.
The remaining 3 warnings are also caused by code which makes it difficult
for the compiler to detect that it is correct.
Regards
Stefan
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH V2] build: remove compile warning
2013-06-22 10:03 ` Stefan Weil
@ 2013-06-24 14:44 ` Paolo Bonzini
2013-06-24 17:58 ` Stefan Weil
2013-06-25 1:50 ` Wenchao Xia
0 siblings, 2 replies; 13+ messages in thread
From: Paolo Bonzini @ 2013-06-24 14:44 UTC (permalink / raw)
To: Stefan Weil
Cc: peter.maydell, aliguori, Wenchao Xia, Markus Armbruster,
qemu-devel, alevy, stefanha, mlureau, Robert Relyea
Il 22/06/2013 12:03, Stefan Weil ha scritto:
> Am 18.06.2013 12:13, schrieb Paolo Bonzini:
>> Il 07/06/2013 14:17, Markus Armbruster ha scritto:
>>>> diff --git a/util/iov.c b/util/iov.c
>>>> index cc6e837..b91cfb9 100644
>>>> --- a/util/iov.c
>>>> +++ b/util/iov.c
>>>> @@ -146,7 +146,7 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt,
>>>> {
>>>> ssize_t total = 0;
>>>> ssize_t ret;
>>>> - size_t orig_len, tail;
>>>> + size_t orig_len = 0, tail;
>>>> unsigned niov;
>>>>
>>>> while (bytes > 0) {
>>> Here are the uses of orig_len:
>>>
>>> if (tail) {
>>> /* second, fixup the last element, and remember the original
>>> * length */
>>> assert(niov < iov_cnt);
>>> assert(iov[niov].iov_len > tail);
>>> orig_len = iov[niov].iov_len;
>>> iov[niov++].iov_len = tail;
>>> }
>>>
>>> ret = do_send_recv(sockfd, iov, niov, do_send);
>>>
>>> /* Undo the changes above before checking for errors */
>>> if (tail) {
>>> iov[niov-1].iov_len = orig_len;
>>> }
>>>
>>>
>
> I get this warning, too, when I run a normal cross compilation with
> MinGW-w64:
>
> util/iov.c:190:33: warning: ‘orig_len’ may be used uninitialized in this
> function [-Wuninitialized]
>
> My build environment:
>
> Debian wheezy with packages gcc-mingw-w64-i686, gcc-mingw-w64-x86-64
> (4.6.3-14+8).
>
> A complete build results in 5 warnings. Here are the other 4 of them:
>
> hw/arm/spitz.c:280:0: warning: "MOD_SHIFT" redefined [enabled by default]
> hw/ppc/spapr.c:673:26: warning: cast from pointer to integer of
> different size [-Wpointer-to-int-cast]
Isn't this one a Win64 bug?
> hw/ppc/spapr_hcall.c:188:1: warning: control reaches end of non-void
> function [-Wreturn-type]
> hw/ppc/spapr_pci.c:454:1: warning: control reaches end of non-void
> function [-Wreturn-type]
I think you could report this to mingw. GCC should handle "if (!0)
foo()" just fine if foo is noreturn, perhaps the "assertion failure"
runtime function is not noreturn in mingw.
> I already sent a patch for the MOD_SHIFT issue.
>
> The remaining 3 warnings are also caused by code which makes it difficult
> for the compiler to detect that it is correct.
Not as hard as this one, though.
Anyway, I would be okay a fix that makes the code easier to follow for
compilers (and doesn't have too much duplication so that humans are also
happy). But adding "= 0" is the worst, because smart compilers will not
detect a human's mistakes anymore.
Paolo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH V2] build: remove compile warning
2013-06-24 14:44 ` Paolo Bonzini
@ 2013-06-24 17:58 ` Stefan Weil
2013-06-26 8:12 ` Paolo Bonzini
2013-06-25 1:50 ` Wenchao Xia
1 sibling, 1 reply; 13+ messages in thread
From: Stefan Weil @ 2013-06-24 17:58 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Alexander Graf, qemu-devel
Am 24.06.2013 16:44, schrieb Paolo Bonzini:
> Il 22/06/2013 12:03, Stefan Weil ha scritto:
>> I get this warning, too, when I run a normal cross compilation with
>> MinGW-w64:
>>
>> util/iov.c:190:33: warning: ‘orig_len’ may be used uninitialized in this
>> function [-Wuninitialized]
>>
>> My build environment:
>>
>> Debian wheezy with packages gcc-mingw-w64-i686, gcc-mingw-w64-x86-64
>> (4.6.3-14+8).
>>
>> A complete build results in 5 warnings. Here are the other 4 of them:
>>
>> hw/arm/spitz.c:280:0: warning: "MOD_SHIFT" redefined [enabled by default]
>> hw/ppc/spapr.c:673:26: warning: cast from pointer to integer of
>> different size [-Wpointer-to-int-cast]
> Isn't this one a Win64 bug?
Yes. http://patchwork.ozlabs.org/patch/252666/ fixes it.
>> hw/ppc/spapr_hcall.c:188:1: warning: control reaches end of non-void
>> function [-Wreturn-type]
>> hw/ppc/spapr_pci.c:454:1: warning: control reaches end of non-void
>> function [-Wreturn-type]
> I think you could report this to mingw. GCC should handle "if (!0)
> foo()" just fine if foo is noreturn, perhaps the "assertion failure"
> runtime function is not noreturn in mingw.
It's a gcc problem. Removing the assertion manually in the source
code and compiling with NDEBUG (which we do by default)results
in the same compiler warning.
I have sent a small patch series which fixes both warnings and
which hopefully matches your criteria for acceptable code changes :-)
See http://patchwork.ozlabs.org/patch/253937/ and two more.
Regards
Stefan
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH V2] build: remove compile warning
2013-06-24 14:44 ` Paolo Bonzini
2013-06-24 17:58 ` Stefan Weil
@ 2013-06-25 1:50 ` Wenchao Xia
1 sibling, 0 replies; 13+ messages in thread
From: Wenchao Xia @ 2013-06-25 1:50 UTC (permalink / raw)
To: Paolo Bonzini
Cc: peter.maydell, aliguori, Stefan Weil, qemu-devel,
Markus Armbruster, alevy, stefanha, mlureau, Robert Relyea
于 2013-6-24 22:44, Paolo Bonzini 写道:
> Il 22/06/2013 12:03, Stefan Weil ha scritto:
>> Am 18.06.2013 12:13, schrieb Paolo Bonzini:
>>> Il 07/06/2013 14:17, Markus Armbruster ha scritto:
>>>>> diff --git a/util/iov.c b/util/iov.c
>>>>> index cc6e837..b91cfb9 100644
>>>>> --- a/util/iov.c
>>>>> +++ b/util/iov.c
>>>>> @@ -146,7 +146,7 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt,
>>>>> {
>>>>> ssize_t total = 0;
>>>>> ssize_t ret;
>>>>> - size_t orig_len, tail;
>>>>> + size_t orig_len = 0, tail;
>>>>> unsigned niov;
>>>>>
>>>>> while (bytes > 0) {
>>>> Here are the uses of orig_len:
>>>>
>>>> if (tail) {
>>>> /* second, fixup the last element, and remember the original
>>>> * length */
>>>> assert(niov < iov_cnt);
>>>> assert(iov[niov].iov_len > tail);
>>>> orig_len = iov[niov].iov_len;
>>>> iov[niov++].iov_len = tail;
>>>> }
>>>>
>>>> ret = do_send_recv(sockfd, iov, niov, do_send);
>>>>
>>>> /* Undo the changes above before checking for errors */
>>>> if (tail) {
>>>> iov[niov-1].iov_len = orig_len;
>>>> }
>>>>
>>>>
>>
>> I get this warning, too, when I run a normal cross compilation with
>> MinGW-w64:
>>
>> util/iov.c:190:33: warning: ‘orig_len’ may be used uninitialized in this
>> function [-Wuninitialized]
>>
>> My build environment:
>>
>> Debian wheezy with packages gcc-mingw-w64-i686, gcc-mingw-w64-x86-64
>> (4.6.3-14+8).
>>
>> A complete build results in 5 warnings. Here are the other 4 of them:
>>
>> hw/arm/spitz.c:280:0: warning: "MOD_SHIFT" redefined [enabled by default]
>> hw/ppc/spapr.c:673:26: warning: cast from pointer to integer of
>> different size [-Wpointer-to-int-cast]
>
> Isn't this one a Win64 bug?
>
>> hw/ppc/spapr_hcall.c:188:1: warning: control reaches end of non-void
>> function [-Wreturn-type]
>> hw/ppc/spapr_pci.c:454:1: warning: control reaches end of non-void
>> function [-Wreturn-type]
>
> I think you could report this to mingw. GCC should handle "if (!0)
> foo()" just fine if foo is noreturn, perhaps the "assertion failure"
> runtime function is not noreturn in mingw.
>
>> I already sent a patch for the MOD_SHIFT issue.
>>
>> The remaining 3 warnings are also caused by code which makes it difficult
>> for the compiler to detect that it is correct.
>
> Not as hard as this one, though.
>
> Anyway, I would be okay a fix that makes the code easier to follow for
> compilers (and doesn't have too much duplication so that humans are also
> happy). But adding "= 0" is the worst, because smart compilers will not
> detect a human's mistakes anymore.
>
> Paolo
>
Hi Palo,
There is V3 remove uninitlized warning without adding "= 0", could
u take a look for it?
http://lists.nongnu.org/archive/html/qemu-devel/2013-06/msg02348.html
--
Best Regards
Wenchao Xia
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH V2] build: remove compile warning
2013-06-24 17:58 ` Stefan Weil
@ 2013-06-26 8:12 ` Paolo Bonzini
0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2013-06-26 8:12 UTC (permalink / raw)
To: Stefan Weil; +Cc: Alexander Graf, qemu-devel
Il 24/06/2013 19:58, Stefan Weil ha scritto:
>>> hw/ppc/spapr_hcall.c:188:1: warning: control reaches end of non-void
>>> function [-Wreturn-type]
>>> hw/ppc/spapr_pci.c:454:1: warning: control reaches end of non-void
>>> function [-Wreturn-type]
>> I think you could report this to mingw. GCC should handle "if (!0)
>> foo()" just fine if foo is noreturn, perhaps the "assertion failure"
>> runtime function is not noreturn in mingw.
>
> It's a gcc problem. Removing the assertion manually in the source
> code and compiling with NDEBUG (which we do by default)results
> in the same compiler warning.
Huh? That seems wrong, assertions are there for a reason...
Paolo
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2013-06-26 8:13 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-07 10:02 [Qemu-devel] [PATCH V2] build: remove compile warning Wenchao Xia
2013-06-07 11:35 ` Stefan Hajnoczi
2013-06-07 12:17 ` Markus Armbruster
2013-06-08 2:04 ` Wenchao Xia
2013-06-18 10:14 ` Paolo Bonzini
2013-06-18 10:13 ` Paolo Bonzini
2013-06-19 6:27 ` Wenchao Xia
2013-06-19 9:33 ` Paolo Bonzini
2013-06-22 10:03 ` Stefan Weil
2013-06-24 14:44 ` Paolo Bonzini
2013-06-24 17:58 ` Stefan Weil
2013-06-26 8:12 ` Paolo Bonzini
2013-06-25 1:50 ` Wenchao Xia
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.