[Qemu-devel] [PATCH for-2.10 3/4] bsd-user/bsdload.c: Remove write-only id_change variable

Peter Maydell posted 4 patches 8 years, 6 months ago
[Qemu-devel] [PATCH for-2.10 3/4] bsd-user/bsdload.c: Remove write-only id_change variable
Posted by Peter Maydell 8 years, 6 months ago
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


Re: [Qemu-devel] [PATCH for-2.10 3/4] bsd-user/bsdload.c: Remove write-only id_change variable
Posted by Thomas Huth 8 years, 6 months ago
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>

Re: [Qemu-devel] [PATCH for-2.10 3/4] bsd-user/bsdload.c: Remove write-only id_change variable
Posted by Peter Maydell 8 years, 6 months ago
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