Re: [Qemu-devel] [PATCH V2] build: remove compile warning
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
Re: [Qemu-devel] [PATCH V2] build: remove compile warning
于 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
Re: [Qemu-devel] [PATCH V2] build: remove compile warning
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
Re: [Qemu-devel] [PATCH V2] build: remove compile warning
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
Re: [Qemu-devel] [PATCH V2] build: remove compile warning
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
Re: [Qemu-devel] [PATCH V2] build: remove compile warning
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
Re: [Qemu-devel] [PATCH V2] build: remove compile warning
于 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
Re: [Qemu-devel] [PATCH V2] build: remove compile warning
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
Re: [Qemu-devel] [PATCH V2] build: remove compile warning
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
Re: [Qemu-devel] [PATCH V2] build: remove compile warning
I insist to remove compile warning since I want gcc check my code with
strict rule.
Wenchao Xia writes:
This patch simply remove "variable may be used uninitialized" warning.
Signed-off-by: Wenchao Xia
---
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
Re: [Qemu-devel] [PATCH V2] build: remove compile warning
Wenchao Xia writes:
> This patch simply remove "variable may be used uninitialized" warning.
>
> Signed-off-by: Wenchao Xia
> ---
> 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);
}
Re: [Qemu-devel] [PATCH V2] build: remove compile warning
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 > --- > 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
[Qemu-devel] [PATCH V2] build: remove compile warning
This patch simply remove "variable may be used uninitialized" warning.
Signed-off-by: Wenchao Xia
---
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
