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
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 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
于 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 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
于 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 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
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
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
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 xiaw...@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 stefa...@redhat.com
Re: [Qemu-devel] [PATCH V2] build: remove compile warning
Wenchao Xia xiaw...@linux.vnet.ibm.com writes:
This patch simply remove variable may be used uninitialized warning.
Signed-off-by: Wenchao Xia xiaw...@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);
}
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 xiaw...@linux.vnet.ibm.com writes:
This patch simply remove variable may be used uninitialized warning.
Signed-off-by: Wenchao Xia xiaw...@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
