Re: [Qemu-devel] [PATCH] linux-user, fix getgroups/getgroups32 when both args are zero.

2008-02-26 Thread Kirill A. Shutemov 
On [Sat, 23.02.2008 09:25], Kirill A. Shutemov wrote:
 On [Sat, 23.02.2008 12:00], Takashi Yoshii wrote:
  getgroups() and getgroups32() returns NGROUPS_MAX when both its two args are
  zero. But because we pass a ptr to allocated space as 2nd arg, this function
  are interfered. The patch attached fixed it.
  /yoshii
  ---
  linux-user/syscall.c: fix getgroups{,32} when both args are zero.
  
 
 My version of patch to fix same problem:
 
 getgroups: return total number of supplementary group IDs for the
 process if size == 0
 
 diff --git a/linux-user/syscall.c b/linux-user/syscall.c
 index ad97871..96a11a9 100644
 --- a/linux-user/syscall.c
 +++ b/linux-user/syscall.c
 @@ -5029,6 +5029,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
 arg1,
  
  grouplist = alloca(gidsetsize * sizeof(gid_t));
  ret = get_errno(getgroups(gidsetsize, grouplist));
 +if (gidsetsize == 0)
 +break;
  if (!is_error(ret)) {
  target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize 
 * 2, 0);
  if (!target_grouplist)
 @@ -5179,6 +5181,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
 arg1,
  
  grouplist = alloca(gidsetsize * sizeof(gid_t));
  ret = get_errno(getgroups(gidsetsize, grouplist));
 +if (gidsetsize == 0)
 +break;
  if (!is_error(ret)) {
  target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize 
 * 4, 0);
  if (!target_grouplist) {
 
 Another patch for getgroups:
 
 Trivial optimization of getgroups syscall implementation:
 swap only returned groups, not all group in list.
 
 diff --git a/linux-user/syscall.c b/linux-user/syscall.c
 index 6f2872f..ad97871 100644
 --- a/linux-user/syscall.c
 +++ b/linux-user/syscall.c
 @@ -5033,7 +5033,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
 arg1,
  target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize 
 * 2, 0);
  if (!target_grouplist)
  goto efault;
 -for(i = 0;i  gidsetsize; i++)
 +for(i = 0;i  ret; i++)
  target_grouplist[i] = tswap16(grouplist[i]);
  unlock_user(target_grouplist, arg2, gidsetsize * 2);
  }
 @@ -5185,7 +5185,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
 arg1,
  ret = -TARGET_EFAULT;
  goto fail;
  }
 -for(i = 0;i  gidsetsize; i++)
 +for(i = 0;i  ret; i++)
  target_grouplist[i] = tswap32(grouplist[i]);
  unlock_user(target_grouplist, arg2, gidsetsize * 4);
  }
 
 It makes getgroups much faster in some cases.

Can anybody commit it?

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + Velesys Ltd, http://www.velesys.com/
 + ALT Linux Team, http://www.altlinux.com/


signature.asc
Description: Digital signature

Re: [Qemu-devel] [PATCH] linux-user, fix getgroups/getgroups32 when both args are zero.

2008-02-23 Thread Takashi Yoshii 
Hi,

 My version of patch to fix same problem:
Oh, yes. You are right.
I have read manpage again, and I found it says,
  if size is zero, list is not modified, but ...
So, mine is wrong. We should check only size.
/yoshii

[Qemu-devel] [PATCH] linux-user, fix getgroups/getgroups32 when both args are zero.

2008-02-22 Thread Takashi Yoshii 
getgroups() and getgroups32() returns NGROUPS_MAX when both its two args are
zero. But because we pass a ptr to allocated space as 2nd arg, this function
are interfered. The patch attached fixed it.
/yoshii
---
linux-user/syscall.c: fix getgroups{,32} when both args are zero.

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5185,7 +5185,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 ret = get_errno(setregid(low2highgid(arg1), low2highgid(arg2)));
 break;
 case TARGET_NR_getgroups:
-{
+if (arg1 == 0  arg2 == 0)
+ret = get_errno(getgroups(0, 0));
+else {
 int gidsetsize = arg1;
 uint16_t *target_grouplist;
 gid_t *grouplist;
@@ -5335,7 +5337,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_getgroups32
 case TARGET_NR_getgroups32:
-{
+if (arg1 == 0  arg2 == 0)
+ret = get_errno(getgroups(0, 0));
+else {
 int gidsetsize = arg1;
 uint32_t *target_grouplist;
 gid_t *grouplist;

Re: [Qemu-devel] [PATCH] linux-user, fix getgroups/getgroups32 when both args are zero.

2008-02-22 Thread Kirill A. Shutemov 
On [Sat, 23.02.2008 12:00], Takashi Yoshii wrote:
 getgroups() and getgroups32() returns NGROUPS_MAX when both its two args are
 zero. But because we pass a ptr to allocated space as 2nd arg, this function
 are interfered. The patch attached fixed it.
 /yoshii
 ---
 linux-user/syscall.c: fix getgroups{,32} when both args are zero.
 

My version of patch to fix same problem:

getgroups: return total number of supplementary group IDs for the
process if size == 0

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ad97871..96a11a9 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5029,6 +5029,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 
 grouplist = alloca(gidsetsize * sizeof(gid_t));
 ret = get_errno(getgroups(gidsetsize, grouplist));
+if (gidsetsize == 0)
+break;
 if (!is_error(ret)) {
 target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * 
2, 0);
 if (!target_grouplist)
@@ -5179,6 +5181,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 
 grouplist = alloca(gidsetsize * sizeof(gid_t));
 ret = get_errno(getgroups(gidsetsize, grouplist));
+if (gidsetsize == 0)
+break;
 if (!is_error(ret)) {
 target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * 
4, 0);
 if (!target_grouplist) {

Another patch for getgroups:

Trivial optimization of getgroups syscall implementation:
swap only returned groups, not all group in list.

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 6f2872f..ad97871 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5033,7 +5033,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * 
2, 0);
 if (!target_grouplist)
 goto efault;
-for(i = 0;i  gidsetsize; i++)
+for(i = 0;i  ret; i++)
 target_grouplist[i] = tswap16(grouplist[i]);
 unlock_user(target_grouplist, arg2, gidsetsize * 2);
 }
@@ -5185,7 +5185,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 ret = -TARGET_EFAULT;
 goto fail;
 }
-for(i = 0;i  gidsetsize; i++)
+for(i = 0;i  ret; i++)
 target_grouplist[i] = tswap32(grouplist[i]);
 unlock_user(target_grouplist, arg2, gidsetsize * 4);
 }

It makes getgroups much faster in some cases.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + Velesys Ltd, http://www.velesys.com/
 + ALT Linux Team, http://www.altlinux.com/


signature.asc
Description: Digital signature