Patchwork Fix a number of unused-but-set-variable warnings (new with gcc-4.6)

login
register
mail settings
Submitter Hans de Goede
Date May 3, 2011, 11:03 a.m.
Message ID <1304420620-8409-1-git-send-email-hdegoede@redhat.com>
Download mbox | patch
Permalink /patch/93763/
State New
Headers show

Comments

Hans de Goede - May 3, 2011, 11:03 a.m.
---
 target-i386/kvm.c |    4 ++--
 tcg/tcg.c         |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
Christophe Fergeau - May 17, 2011, 11:25 a.m.
Hi Hans,

On Tue, May 03, 2011 at 01:03:40PM +0200, Hans de Goede wrote:
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index a13599d..e9e8d54 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -950,7 +950,7 @@ static int kvm_get_xsave(CPUState *env)
> @@ -966,7 +966,7 @@ static int kvm_get_xsave(CPUState *env)
>      cwd = (uint16_t)xsave->region[0];
>      swd = (uint16_t)(xsave->region[0] >> 16);
>      twd = (uint16_t)xsave->region[1];
> -    fop = (uint16_t)(xsave->region[1] >> 16);
> +    /* fop = (uint16_t)(xsave->region[1] >> 16); */

Wouldn't it be better to drop this line?

>      env->fpstt = (swd >> 11) & 7;
>      env->fpus = swd;
>      env->fpuc = cwd;
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 8748c05..11a8daf 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -585,7 +585,7 @@ void tcg_register_helper(void *func, const char *name)
>  void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags,
>                     int sizemask, TCGArg ret, int nargs, TCGArg *args)
>  {
> -#ifdef TCG_TARGET_I386
> +#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 

This function uses #if defined(TCG_TARGET_I386) in other places, so I'd use
parentheses here for consistency.

>      int call_type;
>  #endif
>      int i;
> @@ -612,7 +612,7 @@ void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags,
>  
>      *gen_opc_ptr++ = INDEX_op_call;
>      nparam = gen_opparam_ptr++;
> -#ifdef TCG_TARGET_I386
> +#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 

Same here.

Apart from that, I also need this patch to be able to build qemu on fedora
15 :)

Christophe
Aurelien Jarno - May 17, 2011, 5:51 p.m.
On Tue, May 17, 2011 at 01:25:10PM +0200, Christophe Fergeau wrote:
> Hi Hans,
> 
> On Tue, May 03, 2011 at 01:03:40PM +0200, Hans de Goede wrote:
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index a13599d..e9e8d54 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -950,7 +950,7 @@ static int kvm_get_xsave(CPUState *env)
> > @@ -966,7 +966,7 @@ static int kvm_get_xsave(CPUState *env)
> >      cwd = (uint16_t)xsave->region[0];
> >      swd = (uint16_t)(xsave->region[0] >> 16);
> >      twd = (uint16_t)xsave->region[1];
> > -    fop = (uint16_t)(xsave->region[1] >> 16);
> > +    /* fop = (uint16_t)(xsave->region[1] >> 16); */
> 
> Wouldn't it be better to drop this line?
> 
> >      env->fpstt = (swd >> 11) & 7;
> >      env->fpus = swd;
> >      env->fpuc = cwd;
> > diff --git a/tcg/tcg.c b/tcg/tcg.c
> > index 8748c05..11a8daf 100644
> > --- a/tcg/tcg.c
> > +++ b/tcg/tcg.c
> > @@ -585,7 +585,7 @@ void tcg_register_helper(void *func, const char *name)
> >  void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags,
> >                     int sizemask, TCGArg ret, int nargs, TCGArg *args)
> >  {
> > -#ifdef TCG_TARGET_I386
> > +#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 
> 
> This function uses #if defined(TCG_TARGET_I386) in other places, so I'd use
> parentheses here for consistency.
> 
> >      int call_type;
> >  #endif
> >      int i;
> > @@ -612,7 +612,7 @@ void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags,
> >  
> >      *gen_opc_ptr++ = INDEX_op_call;
> >      nparam = gen_opparam_ptr++;
> > -#ifdef TCG_TARGET_I386
> > +#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 
> 
> Same here.
> 
> Apart from that, I also need this patch to be able to build qemu on fedora
> 15 :)
> 

You should also probably split the patch in two parts, as it touch
totally different parts of QEMU, and thus is going to be
reviewed/applied by different person. I am fine with the TCG part after
the comments from Christophe, so I'll apply it as soon as the new
version is out.
Amit Shah - May 27, 2011, 10:34 a.m.
On (Tue) 03 May 2011 [13:03:40], Hans de Goede wrote:
> ---
>  target-i386/kvm.c |    4 ++--
>  tcg/tcg.c         |    4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)

Thanks; just got hit by this.

However, there are a couple of whitespace issues:

> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -585,7 +585,7 @@ void tcg_register_helper(void *func, const char *name)
>  void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags,
>                     int sizemask, TCGArg ret, int nargs, TCGArg *args)
>  {
> -#ifdef TCG_TARGET_I386
> +#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 
>      int call_type;
>  #endif
>      int i;
> @@ -612,7 +612,7 @@ void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags,
>  
>      *gen_opc_ptr++ = INDEX_op_call;
>      nparam = gen_opparam_ptr++;
> -#ifdef TCG_TARGET_I386
> +#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 
>      call_type = (flags & TCG_CALL_TYPE_MASK);
>  #endif
>      if (ret != TCG_CALL_DUMMY_ARG) {

Both these lines have a trailing space.

Care to resubmit with Anthony in CC?  You can add:

Acked-by: Amit Shah <amit.shah@redhat.com>

		Amit
Christophe Fergeau - May 30, 2011, 10:28 a.m.
Hi,

Here are Hans's patches split and with the various issues that were pointed
out fixed. I'm not sure how I'm supposed to handle acks, original author,
... when reworking patches this way, let me know if I should proceed
differently.

Christophe


Christophe Fergeau (2):
  tcg: Fix unused-but-set-variable warning
  kvm: Fix unused-but-set-variable warning

 target-i386/kvm.c |    3 +--
 tcg/tcg.c         |    4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)
Amit Shah - May 30, 2011, 10:53 a.m.
On (Mon) 30 May 2011 [12:28:01], Christophe Fergeau wrote:
> Hi,
> 
> Here are Hans's patches split and with the various issues that were pointed
> out fixed. I'm not sure how I'm supposed to handle acks, original author,
> ... when reworking patches this way, let me know if I should proceed
> differently.

You should keep From: as the same person, keep his Signed-off-by, add
your Signed-off-by and mention what you have changed in the series.

Thanks for re-sending this, btw.

		Amit
Christophe Fergeau - May 30, 2011, 1:56 p.m.
On Mon, May 30, 2011 at 04:23:43PM +0530, Amit Shah wrote:
> You should keep From: as the same person, keep his Signed-off-by, add
> your Signed-off-by and mention what you have changed in the series.

Hans's patches didn't have a S-o-b. I can resend the same patches again
with my S-o-b (I'm fine with doing this without Hans's since I came up with
exactly the same patches independently from Hans) and detailing the changes
from the initial patch. Does that sound good?

Christophe
Amit Shah - May 31, 2011, 5:43 a.m.
On (Mon) 30 May 2011 [15:56:28], Christophe Fergeau wrote:
> On Mon, May 30, 2011 at 04:23:43PM +0530, Amit Shah wrote:
> > You should keep From: as the same person, keep his Signed-off-by, add
> > your Signed-off-by and mention what you have changed in the series.
> 
> Hans's patches didn't have a S-o-b. I can resend the same patches again
> with my S-o-b (I'm fine with doing this without Hans's since I came up with
> exactly the same patches independently from Hans) and detailing the changes
> from the initial patch. Does that sound good?

Yes, that's fine.

		Amit
Christophe Fergeau - May 31, 2011, 7:53 a.m.
Here is another version of these patches.

Christophe

Changes:
v3
 - added S-o-b

v2
 - split patches
 - removed whitespace at end of lines in tcm.
 - changed #if defined XXX to #if defined(XXX)
 - removed commented out line from kvm.c in the 1st patch

v1
 - initial version from Hans

Christophe Fergeau (2):
  tcg: Fix unused-but-set-variable warning
  kvm: Fix unused-but-set-variable warning

 target-i386/kvm.c |    3 +--
 tcg/tcg.c         |    4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)

Patch

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index a13599d..e9e8d54 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -950,7 +950,7 @@  static int kvm_get_xsave(CPUState *env)
 #ifdef KVM_CAP_XSAVE
     struct kvm_xsave* xsave;
     int ret, i;
-    uint16_t cwd, swd, twd, fop;
+    uint16_t cwd, swd, twd;
 
     if (!kvm_has_xsave()) {
         return kvm_get_fpu(env);
@@ -966,7 +966,7 @@  static int kvm_get_xsave(CPUState *env)
     cwd = (uint16_t)xsave->region[0];
     swd = (uint16_t)(xsave->region[0] >> 16);
     twd = (uint16_t)xsave->region[1];
-    fop = (uint16_t)(xsave->region[1] >> 16);
+    /* fop = (uint16_t)(xsave->region[1] >> 16); */
     env->fpstt = (swd >> 11) & 7;
     env->fpus = swd;
     env->fpuc = cwd;
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 8748c05..11a8daf 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -585,7 +585,7 @@  void tcg_register_helper(void *func, const char *name)
 void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags,
                    int sizemask, TCGArg ret, int nargs, TCGArg *args)
 {
-#ifdef TCG_TARGET_I386
+#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 
     int call_type;
 #endif
     int i;
@@ -612,7 +612,7 @@  void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags,
 
     *gen_opc_ptr++ = INDEX_op_call;
     nparam = gen_opparam_ptr++;
-#ifdef TCG_TARGET_I386
+#if defined TCG_TARGET_I386 && TCG_TARGET_REG_BITS < 64 
     call_type = (flags & TCG_CALL_TYPE_MASK);
 #endif
     if (ret != TCG_CALL_DUMMY_ARG) {