[Qemu-devel] [PATCH 00/14] More gcc 4.6 warnings fixes

[Qemu-devel] [PATCH 00/14] More gcc 4.6 warnings fixes

[Juan Quintela]

Hi

This series fixes compilation of plain:

./configure

with gcc-4.6.0 on Fedora 15.

Notes:
- linuxload.c: why it define id_change if it never test its value?
  git log shows that it has been this way forever
- linux-user/syscall.c: do we want to return an error in the default case?
  my guess is yes, but ...
- mips: it puts 8 arguments on the stack, but do_syscall() only uses 6.
  at least some syscalls uses already 7 arguments, I guess this has never worked before.

- for this kind of warnings, I have added:
   (void)unused_var;
  We can remove the variable altogether, comment it, use __attribute__(no_warn_unused).

- linux-user, syscall for alpha.  can anyone check tat my s/arg1/how/
  is the right change.  Looking at the normal sigprocmask call
  emulation, it looks like my change is wright, but one never knows.

Later, Juan.


Juan Quintela (14):
  kvm: remove fop write only variable
  tcg: define and set call_type only when it is used
  flatload: memp was a write-only variable
  xen: pentry is not used in this function
  linuxload: id_change was a write only variable
  flatload: end_code was only used in a debug message
  alpha: fn2 was a write only variable
  syscall: really return ret code
  exec: last_first_tb was only used in !ONLY_USER case
  mips: we really need the extra arguments
  linux-user: fpu_save_addr is not used
  linux-user: syscall should use sanitized arg1
  alpha: disp12 is not used for USER emulation
  lsi53c895a: current_dev is not used

 exec.c                   |   10 +++++++---
 hw/lsi53c895a.c          |    2 --
 linux-user/flatload.c    |    8 ++------
 linux-user/linuxload.c   |   25 +------------------------
 linux-user/main.c        |    4 ++++
 linux-user/signal.c      |    3 ++-
 linux-user/syscall.c     |   10 +++++-----
 target-alpha/translate.c |   10 +++++++---
 target-i386/kvm.c        |    3 +--
 tcg/tcg.c                |    9 +++------
 xen-mapcache.c           |    3 +--
 11 files changed, 33 insertions(+), 54 deletions(-)

-- 
1.7.5.2

[Qemu-devel] [PATCH 01/14] kvm: remove fop write only variable

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 target-i386/kvm.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index faedc6c..58a70bc 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -970,7 +970,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);
@@ -986,7 +986,6 @@ 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);
     env->fpstt = (swd >> 11) & 7;
     env->fpus = swd;
     env->fpuc = cwd;
-- 
1.7.5.2

[Qemu-devel] [PATCH 02/14] tcg: define and set call_type only when it is used

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 tcg/tcg.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index fad92f9..2d180a5 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -585,9 +585,6 @@ 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
-    int call_type;
-#endif
     int i;
     int real_args;
     int nb_rets;
@@ -612,9 +609,6 @@ 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
-    call_type = (flags & TCG_CALL_TYPE_MASK);
-#endif
     if (ret != TCG_CALL_DUMMY_ARG) {
 #if TCG_TARGET_REG_BITS < 64
         if (sizemask & 1) {
@@ -641,6 +635,9 @@ void tcg_gen_callN(TCGContext *s, TCGv_ptr func, unsigned int flags,
         int is_64bit = sizemask & (1 << (i+1)*2);
         if (is_64bit) {
 #ifdef TCG_TARGET_I386
+            int call_type;
+
+            call_type = (flags & TCG_CALL_TYPE_MASK);
             /* REGPARM case: if the third parameter is 64 bit, it is
                allocated on the stack */
             if (i == 2 && call_type == TCG_CALL_TYPE_REGPARM) {
-- 
1.7.5.2

[Qemu-devel] [PATCH 03/14] flatload: memp was a write-only variable

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 linux-user/flatload.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/linux-user/flatload.c b/linux-user/flatload.c
index cd7af7c..580bc21 100644
--- a/linux-user/flatload.c
+++ b/linux-user/flatload.c
@@ -379,7 +379,6 @@ static int load_flat_file(struct linux_binprm * bprm,
     abi_long result;
     abi_ulong realdatastart = 0;
     abi_ulong text_len, data_len, bss_len, stack_len, flags;
-    abi_ulong memp = 0; /* for finding the brk area */
     abi_ulong extra;
     abi_ulong reloc = 0, rp;
     int i, rev, relocs = 0;
@@ -491,7 +490,6 @@ static int load_flat_file(struct linux_binprm * bprm,
         }

         reloc = datapos + (ntohl(hdr->reloc_start) - text_len);
-        memp = realdatastart;

     } else {

@@ -506,7 +504,6 @@ static int load_flat_file(struct linux_binprm * bprm,
         realdatastart = textpos + ntohl(hdr->data_start);
         datapos = realdatastart + indx_len;
         reloc = (textpos + ntohl(hdr->reloc_start) + indx_len);
-        memp = textpos;

 #ifdef CONFIG_BINFMT_ZFLAT
 #error code needs checking
-- 
1.7.5.2

[Qemu-devel] [PATCH 04/14] xen: pentry is not used in this function

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 xen-mapcache.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/xen-mapcache.c b/xen-mapcache.c
index 349cc62..a2419dc 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -198,7 +198,7 @@ uint8_t *qemu_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size, u

 void qemu_map_cache_unlock(void *buffer)
 {
-    MapCacheEntry *entry = NULL, *pentry = NULL;
+    MapCacheEntry *entry = NULL;
     MapCacheRev *reventry;
     target_phys_addr_t paddr_index;
     int found = 0;
@@ -218,7 +218,6 @@ void qemu_map_cache_unlock(void *buffer)

     entry = &mapcache->entry[paddr_index % mapcache->nr_buckets];
     while (entry && entry->paddr_index != paddr_index) {
-        pentry = entry;
         entry = entry->next;
     }
     if (!entry) {
-- 
1.7.5.2

[Qemu-devel] [PATCH 05/14] linuxload: id_change was a write only variable

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 linux-user/linuxload.c |   25 +------------------------
 1 files changed, 1 insertions(+), 24 deletions(-)

diff --git a/linux-user/linuxload.c b/linux-user/linuxload.c
index ac8c486..62ebc7e 100644
--- a/linux-user/linuxload.c
+++ b/linux-user/linuxload.c
@@ -26,22 +26,6 @@ abi_long memcpy_to_target(abi_ulong dest, const void *src,
     return 0;
 }

-static int in_group_p(gid_t g)
-{
-    /* return TRUE if we're in the specified group, FALSE otherwise */
-    int		ngroup;
-    int		i;
-    gid_t	grouplist[NGROUPS];
-
-    ngroup = getgroups(NGROUPS, grouplist);
-    for(i = 0; i < ngroup; i++) {
-	if(grouplist[i] == g) {
-	    return 1;
-	}
-    }
-    return 0;
-}
-
 static int count(char ** vec)
 {
     int		i;
@@ -57,7 +41,7 @@ static int prepare_binprm(struct linux_binprm *bprm)
 {
     struct stat		st;
     int mode;
-    int retval, id_change;
+    int retval;

     if(fstat(bprm->fd, &st) < 0) {
 	return(-errno);
@@ -73,14 +57,10 @@ static int prepare_binprm(struct linux_binprm *bprm)

     bprm->e_uid = geteuid();
     bprm->e_gid = getegid();
-    id_change = 0;

     /* Set-uid? */
     if(mode & S_ISUID) {
     	bprm->e_uid = st.st_uid;
-	if(bprm->e_uid != geteuid()) {
-	    id_change = 1;
-	}
     }

     /* Set-gid? */
@@ -91,9 +71,6 @@ static int prepare_binprm(struct linux_binprm *bprm)
      */
     if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
 	bprm->e_gid = st.st_gid;
-	if (!in_group_p(bprm->e_gid)) {
-		id_change = 1;
-	}
     }

     retval = read(bprm->fd, bprm->buf, BPRM_BUF_SIZE);
-- 
1.7.5.2

[Qemu-devel] [PATCH 06/14] flatload: end_code was only used in a debug message

[Juan Quintela]

Just unfold its definition in only use.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 linux-user/flatload.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/linux-user/flatload.c b/linux-user/flatload.c
index 580bc21..8dad5df 100644
--- a/linux-user/flatload.c
+++ b/linux-user/flatload.c
@@ -383,7 +383,7 @@ static int load_flat_file(struct linux_binprm * bprm,
     abi_ulong reloc = 0, rp;
     int i, rev, relocs = 0;
     abi_ulong fpos;
-    abi_ulong start_code, end_code;
+    abi_ulong start_code;
     abi_ulong indx_len;

     hdr = ((struct flat_hdr *) bprm->buf);		/* exec-header */
@@ -549,11 +549,10 @@ static int load_flat_file(struct linux_binprm * bprm,

     /* The main program needs a little extra setup in the task structure */
     start_code = textpos + sizeof (struct flat_hdr);
-    end_code = textpos + text_len;

     DBG_FLT("%s %s: TEXT=%x-%x DATA=%x-%x BSS=%x-%x\n",
             id ? "Lib" : "Load", bprm->filename,
-            (int) start_code, (int) end_code,
+            (int) start_code, (int) textpos + text_lon,
             (int) datapos,
             (int) (datapos + data_len),
             (int) (datapos + data_len),
-- 
1.7.5.2

[Qemu-devel] [PATCH 07/14] alpha: fn2 was a write only variable

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 target-alpha/translate.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 456ba51..5c11cf2 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -1469,7 +1469,7 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn)
     uint32_t palcode;
     int32_t disp21, disp16, disp12;
     uint16_t fn11;
-    uint8_t opc, ra, rb, rc, fpfn, fn7, fn2, islit, real_islit;
+    uint8_t opc, ra, rb, rc, fpfn, fn7, islit, real_islit;
     uint8_t lit;
     ExitStatus ret;

@@ -1491,7 +1491,6 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn)
     fn11 = (insn >> 5) & 0x000007FF;
     fpfn = fn11 & 0x3F;
     fn7 = (insn >> 5) & 0x0000007F;
-    fn2 = (insn >> 5) & 0x00000003;
     LOG_DISAS("opc %02x ra %2d rb %2d rc %2d disp16 %6d\n",
               opc, ra, rb, rc, disp16);

-- 
1.7.5.2

[Qemu-devel] [PATCH 08/14] syscall: really return ret code

[Juan Quintela]

We assign ret with the error code, but then return 0 unconditionally.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 linux-user/syscall.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5cb27c7..f3d03b0 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3751,10 +3751,10 @@ static abi_long do_get_thread_area(CPUX86State *env, abi_ulong ptr)
 #ifndef TARGET_ABI32
 static abi_long do_arch_prctl(CPUX86State *env, int code, abi_ulong addr)
 {
-    abi_long ret;
+    abi_long ret = 0;
     abi_ulong val;
     int idx;
-    
+
     switch(code) {
     case TARGET_ARCH_SET_GS:
     case TARGET_ARCH_SET_FS:
@@ -3773,13 +3773,13 @@ static abi_long do_arch_prctl(CPUX86State *env, int code, abi_ulong addr)
             idx = R_FS;
         val = env->segs[idx].base;
         if (put_user(val, addr, abi_ulong))
-            return -TARGET_EFAULT;
+            ret = -TARGET_EFAULT;
         break;
     default:
         ret = -TARGET_EINVAL;
         break;
     }
-    return 0;
+    return ret;
 }
 #endif

-- 
1.7.5.2

[Qemu-devel] [PATCH 09/14] exec: last_first_tb was only used in !ONLY_USER case

[Juan Quintela]

Once there, use a better variable name.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 exec.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/exec.c b/exec.c
index 8529390..4b1afec 100644
--- a/exec.c
+++ b/exec.c
@@ -1208,12 +1208,16 @@ static inline void tb_alloc_page(TranslationBlock *tb,
                                  unsigned int n, tb_page_addr_t page_addr)
 {
     PageDesc *p;
-    TranslationBlock *last_first_tb;
+#ifndef CONFIG_USER_ONLY
+    bool page_already_protected;
+#endif

     tb->page_addr[n] = page_addr;
     p = page_find_alloc(page_addr >> TARGET_PAGE_BITS, 1);
     tb->page_next[n] = p->first_tb;
-    last_first_tb = p->first_tb;
+#ifndef CONFIG_USER_ONLY
+    page_already_protected = p->first_tb != NULL;
+#endif
     p->first_tb = (TranslationBlock *)((long)tb | n);
     invalidate_page_bitmap(p);

@@ -1249,7 +1253,7 @@ static inline void tb_alloc_page(TranslationBlock *tb,
     /* if some code is already present, then the pages are already
        protected. So we handle the case where only the first TB is
        allocated in a physical page */
-    if (!last_first_tb) {
+    if (!page_already_protected) {
         tlb_protect_code(page_addr);
     }
 #endif
-- 
1.7.5.2

[Qemu-devel] [PATCH 10/14] mips: we really need the extra arguments

[Juan Quintela]

I have no clue how/why syscalls with 7 parameters work on mips.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 linux-user/main.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 088def3..d13affa 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2067,6 +2067,10 @@ void cpu_loop(CPUMIPSState *env)
                 default:
                     break;
                 }
+                /* We should change do_syscall to take extra args.
+                   Some syscalls on mips already use 7 args */
+                (void)arg7;
+                (void)arg8;
                 ret = do_syscall(env, env->active_tc.gpr[2],
                                  env->active_tc.gpr[4],
                                  env->active_tc.gpr[5],
-- 
1.7.5.2

[Qemu-devel] [PATCH 11/14] linux-user: fpu_save_addr is not used

[Juan Quintela]

It is only read to set the error code?

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 linux-user/signal.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index c7a375f..edf4cdb 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -2122,7 +2122,7 @@ long do_sigreturn(CPUState *env)
 	}

         err |= __get_user(fpu_save_addr, &sf->fpu_save);
-
+        (void)fpu_save_addr;
         //if (fpu_save)
         //        err |= restore_fpu_state(env, fpu_save);

@@ -2295,6 +2295,7 @@ void sparc64_set_context(CPUSPARCState *env)
                  abi_ulong) != 0)
         goto do_sigsegv;
     err |= __get_user(fenab, &(ucp->tuc_mcontext.mc_fpregs.mcfpu_enab));
+    (void)fenab;
     err |= __get_user(env->fprs, &(ucp->tuc_mcontext.mc_fpregs.mcfpu_fprs));
     {
         uint32_t *src, *dst;
-- 
1.7.5.2

[Qemu-devel] [PATCH 12/14] linux-user: syscall should use sanitized arg1

[Juan Quintela]

Looking at the other architectures, we should be using "how" not "arg1".

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 linux-user/syscall.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f3d03b0..c90fcc2 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7077,7 +7077,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             }
             mask = arg2;
             target_to_host_old_sigset(&set, &mask);
-            sigprocmask(arg1, &set, &oldset);
+            sigprocmask(how, &set, &oldset);
             host_to_target_old_sigset(&mask, &oldset);
             ret = mask;
         }
-- 
1.7.5.2

[Qemu-devel] [PATCH 13/14] alpha: disp12 is not used for USER emulation

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 target-alpha/translate.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 5c11cf2..fd286d8 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -1467,7 +1467,10 @@ static void gen_rx(int ra, int set)
 static ExitStatus translate_one(DisasContext *ctx, uint32_t insn)
 {
     uint32_t palcode;
-    int32_t disp21, disp16, disp12;
+    int32_t disp21, disp16;
+#ifndef CONFIG_USER_ONLY
+    int32_t disp12;
+#endif
     uint16_t fn11;
     uint8_t opc, ra, rb, rc, fpfn, fn7, islit, real_islit;
     uint8_t lit;
@@ -1487,7 +1490,9 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn)
     palcode = insn & 0x03FFFFFF;
     disp21 = ((int32_t)((insn & 0x001FFFFF) << 11)) >> 11;
     disp16 = (int16_t)(insn & 0x0000FFFF);
+#ifndef CONFIG_USER_ONLY
     disp12 = (int32_t)((insn & 0x00000FFF) << 20) >> 20;
+#endif
     fn11 = (insn >> 5) & 0x000007FF;
     fpfn = fn11 & 0x3F;
     fn7 = (insn >> 5) & 0x0000007F;
-- 
1.7.5.2

[Qemu-devel] [PATCH 14/14] lsi53c895a: current_dev is not used

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/lsi53c895a.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 83084b6..90c6cbc 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -889,7 +889,6 @@ static void lsi_do_msgout(LSIState *s)
     uint8_t msg;
     int len;
     uint32_t current_tag;
-    SCSIDevice *current_dev;
     lsi_request *current_req, *p, *p_next;
     int id;

@@ -901,7 +900,6 @@ static void lsi_do_msgout(LSIState *s)
         current_req = lsi_find_by_tag(s, current_tag);
     }
     id = (current_tag >> 8) & 0xf;
-    current_dev = s->bus.devs[id];

     DPRINTF("MSG out len=%d\n", s->dbc);
     while (s->dbc) {
-- 
1.7.5.2

Re: [Qemu-devel] [PATCH 14/14] lsi53c895a: current_dev is not used

[Andreas Färber]

Am 02.06.2011 um 13:53 schrieb Juan Quintela:

>
> Signed-off-by: Juan Quintela <quintela@redhat.com>

See http://patchwork.ozlabs.org/patch/98182/ - ack'ed by Paolo.

Andreas

> ---
> hw/lsi53c895a.c |    2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
> index 83084b6..90c6cbc 100644
> --- a/hw/lsi53c895a.c
> +++ b/hw/lsi53c895a.c
> @@ -889,7 +889,6 @@ static void lsi_do_msgout(LSIState *s)
>     uint8_t msg;
>     int len;
>     uint32_t current_tag;
> -    SCSIDevice *current_dev;
>     lsi_request *current_req, *p, *p_next;
>     int id;
>
> @@ -901,7 +900,6 @@ static void lsi_do_msgout(LSIState *s)
>         current_req = lsi_find_by_tag(s, current_tag);
>     }
>     id = (current_tag >> 8) & 0xf;
> -    current_dev = s->bus.devs[id];
>
>     DPRINTF("MSG out len=%d\n", s->dbc);
>     while (s->dbc) {
> -- 
> 1.7.5.2

Re: [Qemu-devel] [PATCH 14/14] lsi53c895a: current_dev is not used

[Juan Quintela]

Andreas Färber <andreas.faerber@web.de> wrote:
> Am 02.06.2011 um 13:53 schrieb Juan Quintela:
>
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> See http://patchwork.ozlabs.org/patch/98182/ - ack'ed by Paolo.
>
> Andreas

oops, yeap.

This makes everything compiles (not that I care which one gets
integrated).  Just to get the whole thing compiling O:-)

Thanks, Juan.

>
>> ---
>> hw/lsi53c895a.c |    2 --
>> 1 files changed, 0 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
>> index 83084b6..90c6cbc 100644
>> --- a/hw/lsi53c895a.c
>> +++ b/hw/lsi53c895a.c
>> @@ -889,7 +889,6 @@ static void lsi_do_msgout(LSIState *s)
>>     uint8_t msg;
>>     int len;
>>     uint32_t current_tag;
>> -    SCSIDevice *current_dev;
>>     lsi_request *current_req, *p, *p_next;
>>     int id;
>>
>> @@ -901,7 +900,6 @@ static void lsi_do_msgout(LSIState *s)
>>         current_req = lsi_find_by_tag(s, current_tag);
>>     }
>>     id = (current_tag >> 8) & 0xf;
>> -    current_dev = s->bus.devs[id];
>>
>>     DPRINTF("MSG out len=%d\n", s->dbc);
>>     while (s->dbc) {
>> -- 
>> 1.7.5.2
>
<#secure method=pgpmime mode=sign>

Re: [Qemu-devel] [PATCH 00/14] More gcc 4.6 warnings fixes

[Christophe Fergeau]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

Hi,

On Thu, Jun 02, 2011 at 01:53:35PM +0200, Juan Quintela wrote:
> Juan Quintela (14):
>   kvm: remove fop write only variable
>   tcg: define and set call_type only when it is used

These were already sent in this thread:
http://thread.gmane.org/gmane.comp.emulators.qemu/101411

>   linuxload: id_change was a write only variable
>   lsi53c895a: current_dev is not used

And these were sent in
http://thread.gmane.org/gmane.comp.emulators.qemu/104773

Your linux-load patch seems better than the one I proposed though, I didn't
noticed there was an unused function after doing the cleanup.

Christophe

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [Qemu-devel] [PATCH 11/14] linux-user: fpu_save_addr is not used

[Peter Maydell]

On 2 June 2011 12:53, Juan Quintela <quintela@redhat.com> wrote:
> It is only read to set the error code?

>         err |= __get_user(fpu_save_addr, &sf->fpu_save);
> -
> +        (void)fpu_save_addr;

In linux-user __get_user can never generate an error: faults
are always caught by the lock_user_struct() or equivalent call
done beforehand. The error handling is I think a leftover from
code borrowed from the kernel (which does have a __get_user
that might return an error).

So I think the correct fix here is just to remove the __get_user
lines and the variables if they're not used.

-- PMM

Re: [Qemu-devel] [PATCH 11/14] linux-user: fpu_save_addr is not used

[Juan Quintela]

Peter Maydell <peter.maydell@linaro.org> wrote:
> On 2 June 2011 12:53, Juan Quintela <quintela@redhat.com> wrote:
>> It is only read to set the error code?
>
>>         err |= __get_user(fpu_save_addr, &sf->fpu_save);
>> -
>> +        (void)fpu_save_addr;
>
> In linux-user __get_user can never generate an error: faults
> are always caught by the lock_user_struct() or equivalent call
> done beforehand. The error handling is I think a leftover from
> code borrowed from the kernel (which does have a __get_user
> that might return an error).
>
> So I think the correct fix here is just to remove the __get_user
> lines and the variables if they're not used.

Fine with me.  Will do for next series.

Later, Juan.

Re: [Qemu-devel] [PATCH 06/14] flatload: end_code was only used in a debug message

[Peter Maydell]

On 2 June 2011 12:53, Juan Quintela <quintela@redhat.com> wrote:
> -    end_code = textpos + text_len;
>
>     DBG_FLT("%s %s: TEXT=%x-%x DATA=%x-%x BSS=%x-%x\n",
>             id ? "Lib" : "Load", bprm->filename,
> -            (int) start_code, (int) end_code,
> +            (int) start_code, (int) textpos + text_lon,

Typo here, should be "text_len", not "text_lon".

-- PMM

Re: [Qemu-devel] [PATCH 08/14] syscall: really return ret code

[Peter Maydell]

On 2 June 2011 12:53, Juan Quintela <quintela@redhat.com> wrote:
> We assign ret with the error code, but then return 0 unconditionally.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

Re: [Qemu-devel] [PATCH 05/14] linuxload: id_change was a write only variable

[Peter Maydell]

On 2 June 2011 12:53, Juan Quintela <quintela@redhat.com> wrote:
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

It does seem a bit odd that we were carefully calculating
this flag and then ignoring it, but I guess we just have
to treat the reason as lost in the mists of time (as you say,
according to the git history it has always been this way..)

-- PMM

Re: [Qemu-devel] [PATCH 12/14] linux-user: syscall should use sanitized arg1

[Peter Maydell]

On 2 June 2011 12:53, Juan Quintela <quintela@redhat.com> wrote:
> Looking at the other architectures, we should be using "how" not "arg1".
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>

OK as far as it goes, but I think we should also change the
  int how = arg1;

to just 'int how;' while we're cleaning up this chunk of code.

-- PMM

Re: [Qemu-devel] [PATCH 09/14] exec: last_first_tb was only used in !ONLY_USER case

[Peter Maydell]

On 2 June 2011 12:53, Juan Quintela <quintela@redhat.com> wrote:
> Once there, use a better variable name.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

> ---
>  exec.c |   10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 8529390..4b1afec 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1208,12 +1208,16 @@ static inline void tb_alloc_page(TranslationBlock *tb,
>                                  unsigned int n, tb_page_addr_t page_addr)
>  {
>     PageDesc *p;
> -    TranslationBlock *last_first_tb;
> +#ifndef CONFIG_USER_ONLY
> +    bool page_already_protected;
> +#endif
>
>     tb->page_addr[n] = page_addr;
>     p = page_find_alloc(page_addr >> TARGET_PAGE_BITS, 1);
>     tb->page_next[n] = p->first_tb;
> -    last_first_tb = p->first_tb;
> +#ifndef CONFIG_USER_ONLY
> +    page_already_protected = p->first_tb != NULL;
> +#endif
>     p->first_tb = (TranslationBlock *)((long)tb | n);
>     invalidate_page_bitmap(p);
>
> @@ -1249,7 +1253,7 @@ static inline void tb_alloc_page(TranslationBlock *tb,
>     /* if some code is already present, then the pages are already
>        protected. So we handle the case where only the first TB is
>        allocated in a physical page */
> -    if (!last_first_tb) {
> +    if (!page_already_protected) {
>         tlb_protect_code(page_addr);
>     }
>  #endif
> --
> 1.7.5.2
>
>
>



-- 
12345678901234567890123456789012345678901234567890123456789012345678901234567890
         1         2         3         4         5         6         7         8

Re: [Qemu-devel] [PATCH 09/14] exec: last_first_tb was only used in !ONLY_USER case

[Stefan Hajnoczi]

On Thu, Jun 02, 2011 at 01:53:44PM +0200, Juan Quintela wrote:
> Once there, use a better variable name.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  exec.c |   10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)

Thanks, applied to the trivial patches tree:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches

Stefan