[Qemu-devel] Re: [PATCH] warnings in net.c

[Qemu-devel] Re: [PATCH] warnings in net.c

[Jan Kiszka]

Filip Navara wrote:
> On Wed, Jun 17, 2009 at 5:47 PM, Ulrich Hecht <uli@suse.de> wrote:
> 
>> removes unused slirp code and initializes ret in net_client_init()
>>
>> Signed-off-by: Ulrich Hecht <uli@suse.de>
> 
> 
> It is definitely not unused when CONFIG_SLIRP is defined. I'm damn sure I
> use the code and that I was debugging it two days ago.

Right, the patch is incorrect. And even with --disable-slirp, it builds
fine for me as-is. Ulrich, what is your setup (configure parameters etc.)?

Also, the ret initialization appears to be unneeded on first glance. No
compiler around has a problem with it. What is your version / setup
here? And more important: What path requires the initialization (because
it remains unclear if ret=0 is the correct until we know the precise case).

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH] warnings in net.c

[Ulrich Hecht]

On Wednesday 17 June 2009, Jan Kiszka wrote:
> Also, the ret initialization appears to be unneeded on first glance.
> No compiler around has a problem with it. What is your version / setup 
> here? And more important: What path requires the initialization
> (because it remains unclear if ret=0 is the correct until we know the
> precise case).

Honestly, I can't reproduce it. I guess this whole thing is bogus after 
all. I shouldn't get up before 11 AM any more...

CU
Uli

-- 
Heute ist
- Tag der Aussiedlung (in Ägypten)

SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)

[Qemu-devel] Re: [PATCH] warnings in net.c

[Jan Kiszka]

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

Ulrich Hecht wrote:
> On Wednesday 17 June 2009, Filip Navara wrote:
>> On Wed, Jun 17, 2009 at 5:47 PM, Ulrich Hecht <uli@suse.de> wrote:
>>> removes unused slirp code and initializes ret in net_client_init()
>>>
>>> Signed-off-by: Ulrich Hecht <uli@suse.de>
>> It is definitely not unused when CONFIG_SLIRP is defined. I'm damn
>> sure I use the code and that I was debugging it two days ago.
> 
> Er, yes, now that you mention it... I even grepped for net_client_init, 
> but I didn't find it.
> 
> Still, it dies thanks to -Werror with a plain "./configure". I guess 
> framing it with #ifdef CONFIG_SLIRP will do.

It _is_ already framed, and it _does_ build flawlessly with
--disable-slirp here. You must be looking at a different version.

Jan


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

[Qemu-devel] [PATCH 0/4] linux-user syscall bugs

[Ulrich Hecht]

Couple of bugs I ran into while implementing the S/390 target.

Ulrich Hecht (4):
  pipe argument should not be signed
  64-bit clean socketcall syscall
  wrap path for access syscall
  getrlimit conversion mix-up

 linux-user/syscall.c |  138 +++++++++++++++++++++++++-------------------------
 1 files changed, 69 insertions(+), 69 deletions(-)

[Qemu-devel] [PATCH 1/4] pipe argument should not be signed

[Ulrich Hecht]

pipedes is an address, it should not be signed (breaks for addresses
> 0x80000000)

Signed-off-by: Ulrich Hecht <uli@suse.de>
---
 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 11564fd..57bb9a7 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -953,7 +953,7 @@ static abi_long do_pipe2(int host_pipe[], int flags)
 #endif
 }
 
-static abi_long do_pipe(void *cpu_env, int pipedes, int flags)
+static abi_long do_pipe(void *cpu_env, abi_ulong pipedes, int flags)
 {
     int host_pipe[2];
     abi_long ret;
-- 
1.6.2.1

[Qemu-devel] [PATCH] warnings in net.c

[Ulrich Hecht]

removes unused slirp code and initializes ret in net_client_init()

Signed-off-by: Ulrich Hecht <uli@suse.de>
---
 net.c |   51 +--------------------------------------------------
 1 files changed, 1 insertions(+), 50 deletions(-)

diff --git a/net.c b/net.c
index af9de73..91976b9 100644
--- a/net.c
+++ b/net.c
@@ -691,55 +691,6 @@ int slirp_is_inited(void)
     return slirp_inited;
 }
 
-static ssize_t slirp_receive(VLANClientState *vc, const uint8_t *buf, size_t size)
-{
-#ifdef DEBUG_SLIRP
-    printf("slirp input:\n");
-    hex_dump(stdout, buf, size);
-#endif
-    slirp_input(buf, size);
-    return size;
-}
-
-static int slirp_in_use;
-
-static void net_slirp_cleanup(VLANClientState *vc)
-{
-    slirp_in_use = 0;
-}
-
-static int net_slirp_init(VLANState *vlan, const char *model, const char *name,
-                          int restricted, const char *ip)
-{
-    if (slirp_in_use) {
-        /* slirp only supports a single instance so far */
-        return -1;
-    }
-    if (!slirp_inited) {
-        slirp_inited = 1;
-        slirp_init(restricted, ip);
-
-        while (slirp_redirs) {
-            struct slirp_config_str *config = slirp_redirs;
-
-            slirp_redirection(NULL, config->str);
-            slirp_redirs = config->next;
-            qemu_free(config);
-        }
-#ifndef _WIN32
-        if (slirp_smb_export) {
-            slirp_smb(slirp_smb_export);
-        }
-#endif
-    }
-
-    slirp_vc = qemu_new_vlan_client(vlan, model, name, NULL, slirp_receive,
-                                    NULL, net_slirp_cleanup, NULL);
-    slirp_vc->info_str[0] = '\0';
-    slirp_in_use = 1;
-    return 0;
-}
-
 static void net_slirp_redir_print(void *opaque, int is_udp,
                                   struct in_addr *laddr, u_int lport,
                                   struct in_addr *faddr, u_int fport)
@@ -2088,7 +2039,7 @@ int net_client_init(Monitor *mon, const char *device, const char *p)
         "vlan", "name", "fd", NULL
     };
     char buf[1024];
-    int vlan_id, ret;
+    int vlan_id, ret = 0;
     VLANState *vlan;
     char *name = NULL;
 
-- 
1.6.2.1

[Qemu-devel] [PATCH 2/4] 64-bit clean socketcall syscall

[Ulrich Hecht]

makes socketcall 64-bit clean so it works on 64-bit big-endian systems

Signed-off-by: Ulrich Hecht <uli@suse.de>
---
 linux-user/syscall.c |  130 +++++++++++++++++++++++++-------------------------
 1 files changed, 65 insertions(+), 65 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 57bb9a7..e541b0d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1751,11 +1751,11 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
     switch(num) {
     case SOCKOP_socket:
 	{
-            int domain, type, protocol;
+            abi_ulong domain, type, protocol;
 
-            if (get_user_s32(domain, vptr)
-                || get_user_s32(type, vptr + n)
-                || get_user_s32(protocol, vptr + 2 * n))
+            if (get_user_ual(domain, vptr)
+                || get_user_ual(type, vptr + n)
+                || get_user_ual(protocol, vptr + 2 * n))
                 return -TARGET_EFAULT;
 
             ret = do_socket(domain, type, protocol);
@@ -1763,13 +1763,13 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
         break;
     case SOCKOP_bind:
 	{
-            int sockfd;
+            abi_ulong sockfd;
             abi_ulong target_addr;
             socklen_t addrlen;
 
-            if (get_user_s32(sockfd, vptr)
+            if (get_user_ual(sockfd, vptr)
                 || get_user_ual(target_addr, vptr + n)
-                || get_user_u32(addrlen, vptr + 2 * n))
+                || get_user_ual(addrlen, vptr + 2 * n))
                 return -TARGET_EFAULT;
 
             ret = do_bind(sockfd, target_addr, addrlen);
@@ -1777,13 +1777,13 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
         break;
     case SOCKOP_connect:
         {
-            int sockfd;
+            abi_ulong sockfd;
             abi_ulong target_addr;
             socklen_t addrlen;
 
-            if (get_user_s32(sockfd, vptr)
+            if (get_user_ual(sockfd, vptr)
                 || get_user_ual(target_addr, vptr + n)
-                || get_user_u32(addrlen, vptr + 2 * n))
+                || get_user_ual(addrlen, vptr + 2 * n))
                 return -TARGET_EFAULT;
 
             ret = do_connect(sockfd, target_addr, addrlen);
@@ -1791,10 +1791,10 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
         break;
     case SOCKOP_listen:
         {
-            int sockfd, backlog;
+            abi_ulong sockfd, backlog;
 
-            if (get_user_s32(sockfd, vptr)
-                || get_user_s32(backlog, vptr + n))
+            if (get_user_ual(sockfd, vptr)
+                || get_user_ual(backlog, vptr + n))
                 return -TARGET_EFAULT;
 
             ret = get_errno(listen(sockfd, backlog));
@@ -1802,12 +1802,12 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
         break;
     case SOCKOP_accept:
         {
-            int sockfd;
+            abi_ulong sockfd;
             abi_ulong target_addr, target_addrlen;
 
-            if (get_user_s32(sockfd, vptr)
+            if (get_user_ual(sockfd, vptr)
                 || get_user_ual(target_addr, vptr + n)
-                || get_user_u32(target_addrlen, vptr + 2 * n))
+                || get_user_ual(target_addrlen, vptr + 2 * n))
                 return -TARGET_EFAULT;
 
             ret = do_accept(sockfd, target_addr, target_addrlen);
@@ -1815,12 +1815,12 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
         break;
     case SOCKOP_getsockname:
         {
-            int sockfd;
+            abi_ulong sockfd;
             abi_ulong target_addr, target_addrlen;
 
-            if (get_user_s32(sockfd, vptr)
+            if (get_user_ual(sockfd, vptr)
                 || get_user_ual(target_addr, vptr + n)
-                || get_user_u32(target_addrlen, vptr + 2 * n))
+                || get_user_ual(target_addrlen, vptr + 2 * n))
                 return -TARGET_EFAULT;
 
             ret = do_getsockname(sockfd, target_addr, target_addrlen);
@@ -1828,12 +1828,12 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
         break;
     case SOCKOP_getpeername:
         {
-            int sockfd;
+            abi_ulong sockfd;
             abi_ulong target_addr, target_addrlen;
 
-            if (get_user_s32(sockfd, vptr)
+            if (get_user_ual(sockfd, vptr)
                 || get_user_ual(target_addr, vptr + n)
-                || get_user_u32(target_addrlen, vptr + 2 * n))
+                || get_user_ual(target_addrlen, vptr + 2 * n))
                 return -TARGET_EFAULT;
 
             ret = do_getpeername(sockfd, target_addr, target_addrlen);
@@ -1841,12 +1841,12 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
         break;
     case SOCKOP_socketpair:
         {
-            int domain, type, protocol;
+            abi_ulong domain, type, protocol;
             abi_ulong tab;
 
-            if (get_user_s32(domain, vptr)
-                || get_user_s32(type, vptr + n)
-                || get_user_s32(protocol, vptr + 2 * n)
+            if (get_user_ual(domain, vptr)
+                || get_user_ual(type, vptr + n)
+                || get_user_ual(protocol, vptr + 2 * n)
                 || get_user_ual(tab, vptr + 3 * n))
                 return -TARGET_EFAULT;
 
@@ -1855,15 +1855,15 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
         break;
     case SOCKOP_send:
         {
-            int sockfd;
+            abi_ulong sockfd;
             abi_ulong msg;
             size_t len;
-            int flags;
+            abi_ulong flags;
 
-            if (get_user_s32(sockfd, vptr)
+            if (get_user_ual(sockfd, vptr)
                 || get_user_ual(msg, vptr + n)
                 || get_user_ual(len, vptr + 2 * n)
-                || get_user_s32(flags, vptr + 3 * n))
+                || get_user_ual(flags, vptr + 3 * n))
                 return -TARGET_EFAULT;
 
             ret = do_sendto(sockfd, msg, len, flags, 0, 0);
@@ -1871,15 +1871,15 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
         break;
     case SOCKOP_recv:
         {
-            int sockfd;
+            abi_ulong sockfd;
             abi_ulong msg;
             size_t len;
-            int flags;
+            abi_ulong flags;
 
-            if (get_user_s32(sockfd, vptr)
+            if (get_user_ual(sockfd, vptr)
                 || get_user_ual(msg, vptr + n)
                 || get_user_ual(len, vptr + 2 * n)
-                || get_user_s32(flags, vptr + 3 * n))
+                || get_user_ual(flags, vptr + 3 * n))
                 return -TARGET_EFAULT;
 
             ret = do_recvfrom(sockfd, msg, len, flags, 0, 0);
@@ -1887,19 +1887,19 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
         break;
     case SOCKOP_sendto:
         {
-            int sockfd;
+            abi_ulong sockfd;
             abi_ulong msg;
             size_t len;
-            int flags;
+            abi_ulong flags;
             abi_ulong addr;
             socklen_t addrlen;
 
-            if (get_user_s32(sockfd, vptr)
+            if (get_user_ual(sockfd, vptr)
                 || get_user_ual(msg, vptr + n)
                 || get_user_ual(len, vptr + 2 * n)
-                || get_user_s32(flags, vptr + 3 * n)
+                || get_user_ual(flags, vptr + 3 * n)
                 || get_user_ual(addr, vptr + 4 * n)
-                || get_user_u32(addrlen, vptr + 5 * n))
+                || get_user_ual(addrlen, vptr + 5 * n))
                 return -TARGET_EFAULT;
 
             ret = do_sendto(sockfd, msg, len, flags, addr, addrlen);
@@ -1907,19 +1907,19 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
         break;
     case SOCKOP_recvfrom:
         {
-            int sockfd;
+            abi_ulong sockfd;
             abi_ulong msg;
             size_t len;
-            int flags;
+            abi_ulong flags;
             abi_ulong addr;
             socklen_t addrlen;
 
-            if (get_user_s32(sockfd, vptr)
+            if (get_user_ual(sockfd, vptr)
                 || get_user_ual(msg, vptr + n)
                 || get_user_ual(len, vptr + 2 * n)
-                || get_user_s32(flags, vptr + 3 * n)
+                || get_user_ual(flags, vptr + 3 * n)
                 || get_user_ual(addr, vptr + 4 * n)
-                || get_user_u32(addrlen, vptr + 5 * n))
+                || get_user_ual(addrlen, vptr + 5 * n))
                 return -TARGET_EFAULT;
 
             ret = do_recvfrom(sockfd, msg, len, flags, addr, addrlen);
@@ -1927,10 +1927,10 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
         break;
     case SOCKOP_shutdown:
         {
-            int sockfd, how;
+            abi_ulong sockfd, how;
 
-            if (get_user_s32(sockfd, vptr)
-                || get_user_s32(how, vptr + n))
+            if (get_user_ual(sockfd, vptr)
+                || get_user_ual(how, vptr + n))
                 return -TARGET_EFAULT;
 
             ret = get_errno(shutdown(sockfd, how));
@@ -1939,13 +1939,13 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
     case SOCKOP_sendmsg:
     case SOCKOP_recvmsg:
         {
-            int fd;
+            abi_ulong fd;
             abi_ulong target_msg;
-            int flags;
+            abi_ulong flags;
 
-            if (get_user_s32(fd, vptr)
+            if (get_user_ual(fd, vptr)
                 || get_user_ual(target_msg, vptr + n)
-                || get_user_s32(flags, vptr + 2 * n))
+                || get_user_ual(flags, vptr + 2 * n))
                 return -TARGET_EFAULT;
 
             ret = do_sendrecvmsg(fd, target_msg, flags,
@@ -1954,17 +1954,17 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
         break;
     case SOCKOP_setsockopt:
         {
-            int sockfd;
-            int level;
-            int optname;
+            abi_ulong sockfd;
+            abi_ulong level;
+            abi_ulong optname;
             abi_ulong optval;
             socklen_t optlen;
 
-            if (get_user_s32(sockfd, vptr)
-                || get_user_s32(level, vptr + n)
-                || get_user_s32(optname, vptr + 2 * n)
+            if (get_user_ual(sockfd, vptr)
+                || get_user_ual(level, vptr + n)
+                || get_user_ual(optname, vptr + 2 * n)
                 || get_user_ual(optval, vptr + 3 * n)
-                || get_user_u32(optlen, vptr + 4 * n))
+                || get_user_ual(optlen, vptr + 4 * n))
                 return -TARGET_EFAULT;
 
             ret = do_setsockopt(sockfd, level, optname, optval, optlen);
@@ -1972,17 +1972,17 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
         break;
     case SOCKOP_getsockopt:
         {
-            int sockfd;
-            int level;
-            int optname;
+            abi_ulong sockfd;
+            abi_ulong level;
+            abi_ulong optname;
             abi_ulong optval;
             socklen_t optlen;
 
-            if (get_user_s32(sockfd, vptr)
-                || get_user_s32(level, vptr + n)
-                || get_user_s32(optname, vptr + 2 * n)
+            if (get_user_ual(sockfd, vptr)
+                || get_user_ual(level, vptr + n)
+                || get_user_ual(optname, vptr + 2 * n)
                 || get_user_ual(optval, vptr + 3 * n)
-                || get_user_u32(optlen, vptr + 4 * n))
+                || get_user_ual(optlen, vptr + 4 * n))
                 return -TARGET_EFAULT;
 
             ret = do_getsockopt(sockfd, level, optname, optval, optlen);
-- 
1.6.2.1

[Qemu-devel] [PATCH 3/4] wrap path for access syscall

[Ulrich Hecht]

Signed-off-by: Ulrich Hecht <uli@suse.de>
---
 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 e541b0d..498ce49 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4529,7 +4529,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     case TARGET_NR_access:
         if (!(p = lock_user_string(arg1)))
             goto efault;
-        ret = get_errno(access(p, arg2));
+        ret = get_errno(access(path(p), arg2));
         unlock_user(p, arg1, 0);
         break;
 #if defined(TARGET_NR_faccessat) && defined(__NR_faccessat)
-- 
1.6.2.1

[Qemu-devel] [PATCH 4/4] getrlimit conversion mix-up

[Ulrich Hecht]

Fixes getrlimit implementation that overwrote the result of the syscall
instead of converting it

Signed-off-by: Ulrich Hecht <uli@suse.de>
---
 linux-user/syscall.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 498ce49..8e10ea2 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5021,8 +5021,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             if (!is_error(ret)) {
                 if (!lock_user_struct(VERIFY_WRITE, target_rlim, arg2, 0))
                     goto efault;
-                rlim.rlim_cur = tswapl(target_rlim->rlim_cur);
-                rlim.rlim_max = tswapl(target_rlim->rlim_max);
+                target_rlim->rlim_cur = tswapl(rlim.rlim_cur);
+                target_rlim->rlim_max = tswapl(rlim.rlim_max);
                 unlock_user_struct(target_rlim, arg2, 1);
             }
         }
-- 
1.6.2.1

[Qemu-devel] Re: [PATCH] warnings in net.c

[Ulrich Hecht]

On Friday 03 July 2009, Ulrich Hecht wrote:
> removes unused slirp code and initializes ret in net_client_init()

Please ignore this, I forgot to delete it...

CU
Uli

-- 
Heute ist
- Bürgerlicher Feiertag (in der Schweiz (Aargau))
- Unabhängigkeitstag (in Belarus, den USA (19 Staaten), Samoa)

SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)

[Qemu-devel] Re: [PATCH 4/4] getrlimit conversion mix-up

[Riku Voipio]

Thanks, all patches look fine and have been added to linux-user-for-upstream
que.

On Fri, Jul 03, 2009 at 05:09:30PM +0200, Ulrich Hecht wrote:
> Fixes getrlimit implementation that overwrote the result of the syscall
> instead of converting it
> 
> Signed-off-by: Ulrich Hecht <uli@suse.de>
> ---
>  linux-user/syscall.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 498ce49..8e10ea2 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5021,8 +5021,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>              if (!is_error(ret)) {
>                  if (!lock_user_struct(VERIFY_WRITE, target_rlim, arg2, 0))
>                      goto efault;
> -                rlim.rlim_cur = tswapl(target_rlim->rlim_cur);
> -                rlim.rlim_max = tswapl(target_rlim->rlim_max);
> +                target_rlim->rlim_cur = tswapl(rlim.rlim_cur);
> +                target_rlim->rlim_max = tswapl(rlim.rlim_max);
>                  unlock_user_struct(target_rlim, arg2, 1);
>              }
>          }
> -- 
> 1.6.2.1