qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH for-2.10 0/4] bsd-user: silence warnings on OpenBSD
@ 2017-07-18 16:26 Peter Maydell
  2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 1/4] bsd-user/mmap.c: Move __thread attribute to right place Peter Maydell
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Peter Maydell @ 2017-07-18 16:26 UTC (permalink / raw)
  To: qemu-devel, qemu-trivial; +Cc: patches

I've just managed to get an OpenBSD VM into my test set for processing
pull requests. There are a handful of minor compiler warnings in
bsd-user, which this patchset fixes, just to reduce the noise
in my logs.

thanks
-- PMM

Peter Maydell (4):
  bsd-user/mmap.c: Move __thread attribute to right place
  bsd-user/elfload.c: Fix set-but-not-used warnings
  bsd-user/bsdload.c: Remove write-only id_change variable
  bsd-user/main.c: Fix unused variable warning

 bsd-user/bsdload.c | 25 +------------------------
 bsd-user/elfload.c | 10 ++++------
 bsd-user/main.c    |  7 ++++---
 bsd-user/mmap.c    |  2 +-
 4 files changed, 10 insertions(+), 34 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH for-2.10 1/4] bsd-user/mmap.c: Move __thread attribute to right place
  2017-07-18 16:26 [Qemu-devel] [PATCH for-2.10 0/4] bsd-user: silence warnings on OpenBSD Peter Maydell
@ 2017-07-18 16:26 ` Peter Maydell
  2017-07-18 16:45   ` Philippe Mathieu-Daudé
  2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 2/4] bsd-user/elfload.c: Fix set-but-not-used warnings Peter Maydell
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2017-07-18 16:26 UTC (permalink / raw)
  To: qemu-devel, qemu-trivial; +Cc: patches

Avoid a compiler warning on OpenBSD:
bsd-user/mmap.c:28:1: warning: '__thread' is not at beginning of declaration [-Wold-style-declaration]
by moving the __thread attribute to its proper place.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 bsd-user/mmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index 7f2018e..20cd29d 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -25,7 +25,7 @@
 //#define DEBUG_MMAP
 
 static pthread_mutex_t mmap_mutex = PTHREAD_MUTEX_INITIALIZER;
-static int __thread mmap_lock_count;
+static __thread int mmap_lock_count;
 
 void mmap_lock(void)
 {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH for-2.10 2/4] bsd-user/elfload.c: Fix set-but-not-used warnings
  2017-07-18 16:26 [Qemu-devel] [PATCH for-2.10 0/4] bsd-user: silence warnings on OpenBSD Peter Maydell
  2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 1/4] bsd-user/mmap.c: Move __thread attribute to right place Peter Maydell
@ 2017-07-18 16:26 ` Peter Maydell
  2017-07-18 16:44   ` [Qemu-devel] [Qemu-trivial] " Philippe Mathieu-Daudé
  2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 3/4] bsd-user/bsdload.c: Remove write-only id_change variable Peter Maydell
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2017-07-18 16:26 UTC (permalink / raw)
  To: qemu-devel, qemu-trivial; +Cc: patches

Fix various warnings about set-but-not-used variables on OpenBSD:

bsd-user/elfload.c:1158:15: warning: variable 'mapped_addr' set but not used [-Wunused-but-set-variable]
bsd-user/elfload.c:1165:9: warning: variable 'status' set but not used [-Wunused-but-set-variable]
bsd-user/elfload.c:1168:15: warning: variable 'elf_stack' set but not used [-Wunused-but-set-variable]

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 bsd-user/elfload.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
index 41a1309..7cccf3e 100644
--- a/bsd-user/elfload.c
+++ b/bsd-user/elfload.c
@@ -1155,21 +1155,20 @@ int load_elf_binary(struct linux_binprm * bprm, struct target_pt_regs * regs,
     unsigned int interpreter_type = INTERPRETER_NONE;
     unsigned char ibcs2_interpreter;
     int i;
-    abi_ulong mapped_addr;
     struct elf_phdr * elf_ppnt;
     struct elf_phdr *elf_phdata;
     abi_ulong elf_bss, k, elf_brk;
     int retval;
     char * elf_interpreter;
     abi_ulong elf_entry, interp_load_addr = 0;
-    int status;
     abi_ulong start_code, end_code, start_data, end_data;
     abi_ulong reloc_func_desc = 0;
-    abi_ulong elf_stack;
+#ifdef LOW_ELF_STACK
+    abi_ulong elf_stack = ~((abi_ulong)0UL);
+#endif
     char passed_fileno[6];
 
     ibcs2_interpreter = 0;
-    status = 0;
     load_addr = 0;
     load_bias = 0;
     elf_ex = *((struct elfhdr *) bprm->buf);          /* exec-header */
@@ -1221,7 +1220,6 @@ int load_elf_binary(struct linux_binprm * bprm, struct target_pt_regs * regs,
     elf_brk = 0;
 
 
-    elf_stack = ~((abi_ulong)0UL);
     elf_interpreter = NULL;
     start_code = ~((abi_ulong)0UL);
     end_code = 0;
@@ -1546,7 +1544,7 @@ int load_elf_binary(struct linux_binprm * bprm, struct target_pt_regs * regs,
                and some applications "depend" upon this behavior.
                Since we do not have the power to recompile these, we
                emulate the SVr4 behavior.  Sigh.  */
-            mapped_addr = target_mmap(0, qemu_host_page_size, PROT_READ | PROT_EXEC,
+            target_mmap(0, qemu_host_page_size, PROT_READ | PROT_EXEC,
                                       MAP_FIXED | MAP_PRIVATE, -1, 0);
     }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH for-2.10 3/4] bsd-user/bsdload.c: Remove write-only id_change variable
  2017-07-18 16:26 [Qemu-devel] [PATCH for-2.10 0/4] bsd-user: silence warnings on OpenBSD Peter Maydell
  2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 1/4] bsd-user/mmap.c: Move __thread attribute to right place Peter Maydell
  2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 2/4] bsd-user/elfload.c: Fix set-but-not-used warnings Peter Maydell
@ 2017-07-18 16:26 ` Peter Maydell
  2017-07-21 11:12   ` Thomas Huth
  2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 4/4] bsd-user/main.c: Fix unused variable warning Peter Maydell
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2017-07-18 16:26 UTC (permalink / raw)
  To: qemu-devel, qemu-trivial; +Cc: patches

On OpenBSD the compiler complains:
bsd-user/bsdload.c:54:17: warning: variable 'id_change' set but not used [-Wunused-but-set-variable]

This is dead code that was originally copied from linux-user.
We fixed this in linux-user in commit 331c23b5ca44293d1 in 2011;
delete the useless code from bsd-user too.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 bsd-user/bsdload.c | 25 +------------------------
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/bsd-user/bsdload.c b/bsd-user/bsdload.c
index 94eec36..f38c4fa 100644
--- a/bsd-user/bsdload.c
+++ b/bsd-user/bsdload.c
@@ -20,22 +20,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[TARGET_NGROUPS];
-
-    ngroup = getgroups(TARGET_NGROUPS, grouplist);
-    for(i = 0; i < ngroup; i++) {
-        if(grouplist[i] == g) {
-            return 1;
-        }
-    }
-    return 0;
-}
-
 static int count(char ** vec)
 {
     int         i;
@@ -51,7 +35,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);
@@ -67,14 +51,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? */
@@ -85,9 +65,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;
-        }
     }
 
     memset(bprm->buf, 0, sizeof(bprm->buf));
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH for-2.10 4/4] bsd-user/main.c: Fix unused variable warning
  2017-07-18 16:26 [Qemu-devel] [PATCH for-2.10 0/4] bsd-user: silence warnings on OpenBSD Peter Maydell
                   ` (2 preceding siblings ...)
  2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 3/4] bsd-user/bsdload.c: Remove write-only id_change variable Peter Maydell
@ 2017-07-18 16:26 ` Peter Maydell
  2017-07-18 22:01   ` Eric Blake
  2017-07-18 21:29 ` [Qemu-devel] [PATCH for-2.10 0/4] bsd-user: silence warnings on OpenBSD no-reply
  2017-07-21 10:27 ` Peter Maydell
  5 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2017-07-18 16:26 UTC (permalink / raw)
  To: qemu-devel, qemu-trivial; +Cc: patches

On OpenBSD the compiler warns:
bsd-user/main.c:622:21: warning: variable 'sig' set but not used [-Wunused-but-set-variable]

This is because a lot of the signal delivery code is #if-0'd
out as unused. Reshuffle #ifdefs a bit to silence the warning.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 bsd-user/main.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index fa9c012..501e16f 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -619,9 +619,10 @@ void cpu_loop(CPUSPARCState *env)
             break;
         case EXCP_DEBUG:
             {
-                int sig;
-
-                sig = gdb_handlesig(cs, TARGET_SIGTRAP);
+#if 0
+                int sig =
+#endif
+                gdb_handlesig(cs, TARGET_SIGTRAP);
 #if 0
                 if (sig)
                   {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [Qemu-trivial] [PATCH for-2.10 2/4] bsd-user/elfload.c: Fix set-but-not-used warnings
  2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 2/4] bsd-user/elfload.c: Fix set-but-not-used warnings Peter Maydell
@ 2017-07-18 16:44   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-07-18 16:44 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Trivial, qemu-devel@nongnu.org Developers, patches

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Le 18 juil. 2017 1:32 PM, "Peter Maydell" <peter.maydell@linaro.org> a
écrit :

> Fix various warnings about set-but-not-used variables on OpenBSD:
>
> bsd-user/elfload.c:1158:15: warning: variable 'mapped_addr' set but not
> used [-Wunused-but-set-variable]
> bsd-user/elfload.c:1165:9: warning: variable 'status' set but not used
> [-Wunused-but-set-variable]
> bsd-user/elfload.c:1168:15: warning: variable 'elf_stack' set but not used
> [-Wunused-but-set-variable]
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  bsd-user/elfload.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
> index 41a1309..7cccf3e 100644
> --- a/bsd-user/elfload.c
> +++ b/bsd-user/elfload.c
> @@ -1155,21 +1155,20 @@ int load_elf_binary(struct linux_binprm * bprm,
> struct target_pt_regs * regs,
>      unsigned int interpreter_type = INTERPRETER_NONE;
>      unsigned char ibcs2_interpreter;
>      int i;
> -    abi_ulong mapped_addr;
>      struct elf_phdr * elf_ppnt;
>      struct elf_phdr *elf_phdata;
>      abi_ulong elf_bss, k, elf_brk;
>      int retval;
>      char * elf_interpreter;
>      abi_ulong elf_entry, interp_load_addr = 0;
> -    int status;
>      abi_ulong start_code, end_code, start_data, end_data;
>      abi_ulong reloc_func_desc = 0;
> -    abi_ulong elf_stack;
> +#ifdef LOW_ELF_STACK
> +    abi_ulong elf_stack = ~((abi_ulong)0UL);
> +#endif
>      char passed_fileno[6];
>
>      ibcs2_interpreter = 0;
> -    status = 0;
>      load_addr = 0;
>      load_bias = 0;
>      elf_ex = *((struct elfhdr *) bprm->buf);          /* exec-header */
> @@ -1221,7 +1220,6 @@ int load_elf_binary(struct linux_binprm * bprm,
> struct target_pt_regs * regs,
>      elf_brk = 0;
>
>
> -    elf_stack = ~((abi_ulong)0UL);
>      elf_interpreter = NULL;
>      start_code = ~((abi_ulong)0UL);
>      end_code = 0;
> @@ -1546,7 +1544,7 @@ int load_elf_binary(struct linux_binprm * bprm,
> struct target_pt_regs * regs,
>                 and some applications "depend" upon this behavior.
>                 Since we do not have the power to recompile these, we
>                 emulate the SVr4 behavior.  Sigh.  */
> -            mapped_addr = target_mmap(0, qemu_host_page_size, PROT_READ |
> PROT_EXEC,
> +            target_mmap(0, qemu_host_page_size, PROT_READ | PROT_EXEC,
>                                        MAP_FIXED | MAP_PRIVATE, -1, 0);
>      }
>
> --
> 2.7.4
>
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.10 1/4] bsd-user/mmap.c: Move __thread attribute to right place
  2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 1/4] bsd-user/mmap.c: Move __thread attribute to right place Peter Maydell
@ 2017-07-18 16:45   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-07-18 16:45 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Trivial, qemu-devel@nongnu.org Developers, patches

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Le 18 juil. 2017 1:29 PM, "Peter Maydell" <peter.maydell@linaro.org> a
écrit :

> Avoid a compiler warning on OpenBSD:
> bsd-user/mmap.c:28:1: warning: '__thread' is not at beginning of
> declaration [-Wold-style-declaration]
> by moving the __thread attribute to its proper place.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  bsd-user/mmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index 7f2018e..20cd29d 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -25,7 +25,7 @@
>  //#define DEBUG_MMAP
>
>  static pthread_mutex_t mmap_mutex = PTHREAD_MUTEX_INITIALIZER;
> -static int __thread mmap_lock_count;
> +static __thread int mmap_lock_count;
>
>  void mmap_lock(void)
>  {
> --
> 2.7.4
>
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.10 0/4] bsd-user: silence warnings on OpenBSD
  2017-07-18 16:26 [Qemu-devel] [PATCH for-2.10 0/4] bsd-user: silence warnings on OpenBSD Peter Maydell
                   ` (3 preceding siblings ...)
  2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 4/4] bsd-user/main.c: Fix unused variable warning Peter Maydell
@ 2017-07-18 21:29 ` no-reply
  2017-07-21 10:27 ` Peter Maydell
  5 siblings, 0 replies; 16+ messages in thread
From: no-reply @ 2017-07-18 21:29 UTC (permalink / raw)
  To: peter.maydell; +Cc: famz, qemu-devel, qemu-trivial, patches

Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH for-2.10 0/4] bsd-user: silence warnings on OpenBSD
Message-id: 1500395194-21455-1-git-send-email-peter.maydell@linaro.org
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
2205391 bsd-user/main.c: Fix unused variable warning
72ca90a bsd-user/bsdload.c: Remove write-only id_change variable
8d5f872 bsd-user/elfload.c: Fix set-but-not-used warnings
684257a bsd-user/mmap.c: Move __thread attribute to right place

=== OUTPUT BEGIN ===
Checking PATCH 1/4: bsd-user/mmap.c: Move __thread attribute to right place...
Checking PATCH 2/4: bsd-user/elfload.c: Fix set-but-not-used warnings...
Checking PATCH 3/4: bsd-user/bsdload.c: Remove write-only id_change variable...
Checking PATCH 4/4: bsd-user/main.c: Fix unused variable warning...
ERROR: if this code is redundant consider removing it
#26: FILE: bsd-user/main.c:622:
+#if 0

total: 1 errors, 0 warnings, 13 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.10 4/4] bsd-user/main.c: Fix unused variable warning
  2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 4/4] bsd-user/main.c: Fix unused variable warning Peter Maydell
@ 2017-07-18 22:01   ` Eric Blake
  2017-07-19  8:19     ` Peter Maydell
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Blake @ 2017-07-18 22:01 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, qemu-trivial; +Cc: patches

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

On 07/18/2017 11:26 AM, Peter Maydell wrote:
> On OpenBSD the compiler warns:
> bsd-user/main.c:622:21: warning: variable 'sig' set but not used [-Wunused-but-set-variable]
> 
> This is because a lot of the signal delivery code is #if-0'd
> out as unused. Reshuffle #ifdefs a bit to silence the warning.

Why not just nuke the #if 0 code instead (we can always 'git revert' it
later as a starting point for someone that wants to implement it).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.10 4/4] bsd-user/main.c: Fix unused variable warning
  2017-07-18 22:01   ` Eric Blake
@ 2017-07-19  8:19     ` Peter Maydell
  2017-07-21 11:18       ` Thomas Huth
  2017-07-21 12:23       ` Eric Blake
  0 siblings, 2 replies; 16+ messages in thread
From: Peter Maydell @ 2017-07-19  8:19 UTC (permalink / raw)
  To: Eric Blake; +Cc: QEMU Developers, QEMU Trivial, patches@linaro.org

On 18 July 2017 at 23:01, Eric Blake <eblake@redhat.com> wrote:
> On 07/18/2017 11:26 AM, Peter Maydell wrote:
>> On OpenBSD the compiler warns:
>> bsd-user/main.c:622:21: warning: variable 'sig' set but not used [-Wunused-but-set-variable]
>>
>> This is because a lot of the signal delivery code is #if-0'd
>> out as unused. Reshuffle #ifdefs a bit to silence the warning.
>
> Why not just nuke the #if 0 code instead (we can always 'git revert' it
> later as a starting point for someone that wants to implement it).

I went for the minimal change, especially given we know
that the freebsd folks have a big patchset on top of
us fixing a lot of bsd-user code and it didn't seem worth
giving them a more awkward rebase task.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.10 0/4] bsd-user: silence warnings on OpenBSD
  2017-07-18 16:26 [Qemu-devel] [PATCH for-2.10 0/4] bsd-user: silence warnings on OpenBSD Peter Maydell
                   ` (4 preceding siblings ...)
  2017-07-18 21:29 ` [Qemu-devel] [PATCH for-2.10 0/4] bsd-user: silence warnings on OpenBSD no-reply
@ 2017-07-21 10:27 ` Peter Maydell
  5 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2017-07-21 10:27 UTC (permalink / raw)
  To: QEMU Developers, QEMU Trivial
  Cc: patches@linaro.org, Philippe Mathieu-Daudé, Eric Blake

On 18 July 2017 at 17:26, Peter Maydell <peter.maydell@linaro.org> wrote:
> I've just managed to get an OpenBSD VM into my test set for processing
> pull requests. There are a handful of minor compiler warnings in
> bsd-user, which this patchset fixes, just to reduce the noise
> in my logs.
>
> thanks
> -- PMM
>
> Peter Maydell (4):
>   bsd-user/mmap.c: Move __thread attribute to right place
>   bsd-user/elfload.c: Fix set-but-not-used warnings
>   bsd-user/bsdload.c: Remove write-only id_change variable
>   bsd-user/main.c: Fix unused variable warning

I've applied patches 1 and 2 to master; patch 3 still awaiting
review and 4 review/reply to my response-to-review...

thanks
-- PMM

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.10 3/4] bsd-user/bsdload.c: Remove write-only id_change variable
  2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 3/4] bsd-user/bsdload.c: Remove write-only id_change variable Peter Maydell
@ 2017-07-21 11:12   ` Thomas Huth
  2017-07-21 15:25     ` Peter Maydell
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Huth @ 2017-07-21 11:12 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel, qemu-trivial; +Cc: patches, Juan Quintela

On 18.07.2017 18:26, Peter Maydell wrote:
> On OpenBSD the compiler complains:
> bsd-user/bsdload.c:54:17: warning: variable 'id_change' set but not used [-Wunused-but-set-variable]
> 
> This is dead code that was originally copied from linux-user.
> We fixed this in linux-user in commit 331c23b5ca44293d1 in 2011;
> delete the useless code from bsd-user too.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  bsd-user/bsdload.c | 25 +------------------------
>  1 file changed, 1 insertion(+), 24 deletions(-)
> 
> diff --git a/bsd-user/bsdload.c b/bsd-user/bsdload.c
> index 94eec36..f38c4fa 100644
> --- a/bsd-user/bsdload.c
> +++ b/bsd-user/bsdload.c
> @@ -20,22 +20,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[TARGET_NGROUPS];
> -
> -    ngroup = getgroups(TARGET_NGROUPS, grouplist);
> -    for(i = 0; i < ngroup; i++) {
> -        if(grouplist[i] == g) {
> -            return 1;
> -        }
> -    }
> -    return 0;
> -}
> -
>  static int count(char ** vec)
>  {
>      int         i;
> @@ -51,7 +35,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);
> @@ -67,14 +51,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? */
> @@ -85,9 +65,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;
> -        }
>      }
>  
>      memset(bprm->buf, 0, sizeof(bprm->buf));

Matches the commit from linux-user, and looks sane to me, so:

Reviewed-by: Thomas Huth <thuth@redhat.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.10 4/4] bsd-user/main.c: Fix unused variable warning
  2017-07-19  8:19     ` Peter Maydell
@ 2017-07-21 11:18       ` Thomas Huth
  2017-07-21 12:23       ` Eric Blake
  1 sibling, 0 replies; 16+ messages in thread
From: Thomas Huth @ 2017-07-21 11:18 UTC (permalink / raw)
  To: Peter Maydell, Eric Blake
  Cc: QEMU Trivial, QEMU Developers, patches@linaro.org

On 19.07.2017 10:19, Peter Maydell wrote:
> On 18 July 2017 at 23:01, Eric Blake <eblake@redhat.com> wrote:
>> On 07/18/2017 11:26 AM, Peter Maydell wrote:
>>> On OpenBSD the compiler warns:
>>> bsd-user/main.c:622:21: warning: variable 'sig' set but not used [-Wunused-but-set-variable]
>>>
>>> This is because a lot of the signal delivery code is #if-0'd
>>> out as unused. Reshuffle #ifdefs a bit to silence the warning.
>>
>> Why not just nuke the #if 0 code instead (we can always 'git revert' it
>> later as a starting point for someone that wants to implement it).
> 
> I went for the minimal change, especially given we know
> that the freebsd folks have a big patchset on top of
> us fixing a lot of bsd-user code and it didn't seem worth
> giving them a more awkward rebase task.

That's fair. So FWIW:

Reviewed-by: Thomas Huth <thuth@redhat.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.10 4/4] bsd-user/main.c: Fix unused variable warning
  2017-07-19  8:19     ` Peter Maydell
  2017-07-21 11:18       ` Thomas Huth
@ 2017-07-21 12:23       ` Eric Blake
  2017-07-21 15:25         ` Peter Maydell
  1 sibling, 1 reply; 16+ messages in thread
From: Eric Blake @ 2017-07-21 12:23 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers, QEMU Trivial, patches@linaro.org

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

On 07/19/2017 03:19 AM, Peter Maydell wrote:
> On 18 July 2017 at 23:01, Eric Blake <eblake@redhat.com> wrote:
>> On 07/18/2017 11:26 AM, Peter Maydell wrote:
>>> On OpenBSD the compiler warns:
>>> bsd-user/main.c:622:21: warning: variable 'sig' set but not used [-Wunused-but-set-variable]
>>>
>>> This is because a lot of the signal delivery code is #if-0'd
>>> out as unused. Reshuffle #ifdefs a bit to silence the warning.
>>
>> Why not just nuke the #if 0 code instead (we can always 'git revert' it
>> later as a starting point for someone that wants to implement it).
> 
> I went for the minimal change, especially given we know
> that the freebsd folks have a big patchset on top of
> us fixing a lot of bsd-user code and it didn't seem worth
> giving them a more awkward rebase task.

Perhaps worth mentioning in the commit message that keeping the #if 0 is
intentional, then. At any rate,

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.10 3/4] bsd-user/bsdload.c: Remove write-only id_change variable
  2017-07-21 11:12   ` Thomas Huth
@ 2017-07-21 15:25     ` Peter Maydell
  0 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2017-07-21 15:25 UTC (permalink / raw)
  To: Thomas Huth
  Cc: QEMU Developers, QEMU Trivial, patches@linaro.org, Juan Quintela

On 21 July 2017 at 12:12, Thomas Huth <thuth@redhat.com> wrote:
> On 18.07.2017 18:26, Peter Maydell wrote:
>> On OpenBSD the compiler complains:
>> bsd-user/bsdload.c:54:17: warning: variable 'id_change' set but not used [-Wunused-but-set-variable]
>>
>> This is dead code that was originally copied from linux-user.
>> We fixed this in linux-user in commit 331c23b5ca44293d1 in 2011;
>> delete the useless code from bsd-user too.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---

>
> Matches the commit from linux-user, and looks sane to me, so:
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thanks; applied to master.

-- PMM

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.10 4/4] bsd-user/main.c: Fix unused variable warning
  2017-07-21 12:23       ` Eric Blake
@ 2017-07-21 15:25         ` Peter Maydell
  0 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2017-07-21 15:25 UTC (permalink / raw)
  To: Eric Blake; +Cc: QEMU Developers, QEMU Trivial, patches@linaro.org

On 21 July 2017 at 13:23, Eric Blake <eblake@redhat.com> wrote:
> On 07/19/2017 03:19 AM, Peter Maydell wrote:
>> On 18 July 2017 at 23:01, Eric Blake <eblake@redhat.com> wrote:
>>> On 07/18/2017 11:26 AM, Peter Maydell wrote:
>>>> On OpenBSD the compiler warns:
>>>> bsd-user/main.c:622:21: warning: variable 'sig' set but not used [-Wunused-but-set-variable]
>>>>
>>>> This is because a lot of the signal delivery code is #if-0'd
>>>> out as unused. Reshuffle #ifdefs a bit to silence the warning.
>>>
>>> Why not just nuke the #if 0 code instead (we can always 'git revert' it
>>> later as a starting point for someone that wants to implement it).
>>
>> I went for the minimal change, especially given we know
>> that the freebsd folks have a big patchset on top of
>> us fixing a lot of bsd-user code and it didn't seem worth
>> giving them a more awkward rebase task.
>
> Perhaps worth mentioning in the commit message that keeping the #if 0 is
> intentional, then. At any rate,
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks; applied to master with an updated commit message.


-- PMM

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-07-21 15:25 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-18 16:26 [Qemu-devel] [PATCH for-2.10 0/4] bsd-user: silence warnings on OpenBSD Peter Maydell
2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 1/4] bsd-user/mmap.c: Move __thread attribute to right place Peter Maydell
2017-07-18 16:45   ` Philippe Mathieu-Daudé
2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 2/4] bsd-user/elfload.c: Fix set-but-not-used warnings Peter Maydell
2017-07-18 16:44   ` [Qemu-devel] [Qemu-trivial] " Philippe Mathieu-Daudé
2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 3/4] bsd-user/bsdload.c: Remove write-only id_change variable Peter Maydell
2017-07-21 11:12   ` Thomas Huth
2017-07-21 15:25     ` Peter Maydell
2017-07-18 16:26 ` [Qemu-devel] [PATCH for-2.10 4/4] bsd-user/main.c: Fix unused variable warning Peter Maydell
2017-07-18 22:01   ` Eric Blake
2017-07-19  8:19     ` Peter Maydell
2017-07-21 11:18       ` Thomas Huth
2017-07-21 12:23       ` Eric Blake
2017-07-21 15:25         ` Peter Maydell
2017-07-18 21:29 ` [Qemu-devel] [PATCH for-2.10 0/4] bsd-user: silence warnings on OpenBSD no-reply
2017-07-21 10:27 ` Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).