diff mbox

[v4] linux-user/syscall.c: malloc()/calloc() to g_malloc()/g_try_malloc()/g_new0()

Message ID CAPtdW17fGpP_DUMJWddv+W4ThfC96RVSb2QDRg8TdypEbvmQdw@mail.gmail.com
State New
Headers show

Commit Message

Harmandeep Kaur Oct. 5, 2015, 3:32 a.m. UTC 
Convert malloc()/calloc() calls to g_malloc()/g_try_malloc()/g_new0()
in linux-user/syscall.c file

Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
---
v1->v2  convert the free() call in host_to_target_semarray()
to g_free() and calls g_try_malloc(count)  instead of
g_try_malloc(sizeof(count))

v2->v3 used g_try_new() and friends to avoid overflow issues

v3->v4 use g_free for unlock_iovec() and host_to_target_semarray().

 linux-user/syscall.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

         return NULL;
@@ -1950,7 +1950,7 @@ static struct iovec *lock_iovec(int type, abi_ulong
target_addr,
     }
     unlock_user(target_vec, target_addr, 0);
  fail2:
-    free(vec);
+    g_free(vec);
     errno = err;
     return NULL;
 }
@@ -1975,7 +1975,7 @@ static void unlock_iovec(struct iovec *vec, abi_ulong
target_addr,
         unlock_user(target_vec, target_addr, 0);
     }

-    free(vec);
+    g_free(vec);
 }

 static inline int target_to_host_sock_type(int *type)
@@ -2677,14 +2677,14 @@ static inline abi_long target_to_host_semarray(int
semid, unsigned short **host_

     nsems = semid_ds.sem_nsems;

-    *host_array = malloc(nsems*sizeof(unsigned short));
+    *host_array = g_try_new(unsigned short, nsems);
     if (!*host_array) {
         return -TARGET_ENOMEM;
     }
     array = lock_user(VERIFY_READ, target_addr,
                       nsems*sizeof(unsigned short), 1);
     if (!array) {
-        free(*host_array);
+        g_free(*host_array);
         return -TARGET_EFAULT;
     }

@@ -2721,7 +2721,7 @@ static inline abi_long host_to_target_semarray(int
semid, abi_ulong target_addr,
     for(i=0; i<nsems; i++) {
         __put_user((*host_array)[i], &array[i]);
     }
-    free(*host_array);
+    g_free(*host_array);
     unlock_user(array, target_addr, 1);

     return 0;
@@ -2980,7 +2980,7 @@ static inline abi_long do_msgsnd(int msqid, abi_long
msgp,

     if (!lock_user_struct(VERIFY_READ, target_mb, msgp, 0))
         return -TARGET_EFAULT;
-    host_mb = malloc(msgsz+sizeof(long));
+    host_mb = g_try_malloc(msgsz + sizeof(long));
     if (!host_mb) {
         unlock_user_struct(target_mb, msgp, 0);
         return -TARGET_ENOMEM;
@@ -2988,7 +2988,7 @@ static inline abi_long do_msgsnd(int msqid, abi_long
msgp,
     host_mb->mtype = (abi_long) tswapal(target_mb->mtype);
     memcpy(host_mb->mtext, target_mb->mtext, msgsz);
     ret = get_errno(msgsnd(msqid, host_mb, msgsz, msgflg));
-    free(host_mb);
+    g_free(host_mb);
     unlock_user_struct(target_mb, msgp, 0);

     return ret;
@@ -7723,7 +7723,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
             struct linux_dirent *dirp;
             abi_long count = arg3;

-        dirp = malloc(count);
+        dirp = g_try_malloc(sizeof(count));
         if (!dirp) {
                 ret = -TARGET_ENOMEM;
                 goto fail;
@@ -7760,7 +7760,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
         ret = count1;
                 unlock_user(target_dirp, arg2, ret);
             }
-        free(dirp);
+        g_free(dirp);
         }
 #else
         {

Comments

Stefan Hajnoczi Oct. 5, 2015, 8:10 a.m. UTC | #1 
On Mon, Oct 5, 2015 at 4:32 AM, Harmandeep Kaur
<write.harmandeep@gmail.com> wrote:
> Convert malloc()/calloc() calls to g_malloc()/g_try_malloc()/g_new0()
> in linux-user/syscall.c file
>
> Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
> ---
> v1->v2  convert the free() call in host_to_target_semarray()
> to g_free() and calls g_try_malloc(count)  instead of
> g_try_malloc(sizeof(count))
>
> v2->v3 used g_try_new() and friends to avoid overflow issues
>
> v3->v4 use g_free for unlock_iovec() and host_to_target_semarray().
>
>  linux-user/syscall.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Riku Voipio Oct. 6, 2015, 10:16 a.m. UTC | #2 
On maanantaina 5. lokakuuta 2015 6.32.27 EEST, Harmandeep Kaur wrote:
> Convert malloc()/calloc() calls to g_malloc()/g_try_malloc()/g_new0()
> in linux-user/syscall.c file

This message explains what is changed, but for the commit message
we want to know why. A short explanation why this is needed would be
nice.

The patch doesn't apply for me. Check that you are using latest upstream
and try sending with "git send-email" to ensure no formatting by mailer.

One malloc and free was missed, it is in do_ioctl_fs_ioc_fiemap() function.

> Signed-off-by: Harmandeep Kaur <write.harmandeep@gmail.com>
> ---
> v1->v2  convert the free() call in host_to_target_semarray()
> to g_free() and calls g_try_malloc(count)  instead of 
> g_try_malloc(sizeof(count))
>
> v2->v3 used g_try_new() and friends to avoid overflow issues
>
> v3->v4 use g_free for unlock_iovec() and host_to_target_semarray().
>
>  linux-user/syscall.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 98b5766..6e90141 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1559,7 +1559,7 @@ set_timeout:
>                  }
>  
>                  fprog.len = tswap16(tfprog->len);
> -                filter = malloc(fprog.len * sizeof(*filter));
> +                filter = g_try_new(struct sock_filter, fprog.len);
>                  if (filter == NULL) {
>                      unlock_user_struct(tfilter, tfprog->filter, 1);
>                      unlock_user_struct(tfprog, optval_addr, 1);
> @@ -1575,7 +1575,7 @@ set_timeout:
>  
>                  ret = get_errno(setsockopt(sockfd, SOL_SOCKET,
>                                  SO_ATTACH_FILTER, &fprog, sizeof(fprog)));
> -                free(filter);
> +                g_free(filter);
>  
>                  unlock_user_struct(tfilter, tfprog->filter, 1);
>                  unlock_user_struct(tfprog, optval_addr, 1);
> @@ -1886,7 +1886,7 @@ static struct iovec *lock_iovec(int type, 
> abi_ulong target_addr,
>          return NULL;
>      }
>  
> -    vec = calloc(count, sizeof(struct iovec));
> +    vec = g_try_new0(struct iovec, count);
>      if (vec == NULL) {
>          errno = ENOMEM;
>          return NULL;
> @@ -1950,7 +1950,7 @@ static struct iovec *lock_iovec(int type, 
> abi_ulong target_addr,
>      }
>      unlock_user(target_vec, target_addr, 0);
>   fail2:
> -    free(vec);
> +    g_free(vec);
>      errno = err;
>      return NULL;
>  }
> @@ -1975,7 +1975,7 @@ static void unlock_iovec(struct iovec 
> *vec, abi_ulong target_addr,
>          unlock_user(target_vec, target_addr, 0);
>      }
>  
> -    free(vec);
> +    g_free(vec);
>  }
>  
>  static inline int target_to_host_sock_type(int *type)
> @@ -2677,14 +2677,14 @@ static inline abi_long 
> target_to_host_semarray(int semid, unsigned short **host_
>  
>      nsems = semid_ds.sem_nsems;
>  
> -    *host_array = malloc(nsems*sizeof(unsigned short));
> +    *host_array = g_try_new(unsigned short, nsems);
>      if (!*host_array) {
>          return -TARGET_ENOMEM;
>      }
>      array = lock_user(VERIFY_READ, target_addr,
>                        nsems*sizeof(unsigned short), 1);
>      if (!array) {
> -        free(*host_array);
> +        g_free(*host_array);
>          return -TARGET_EFAULT;
>      }
>  
> @@ -2721,7 +2721,7 @@ static inline abi_long 
> host_to_target_semarray(int semid, abi_ulong target_addr,
>      for(i=0; i<nsems; i++) {
>          __put_user((*host_array)[i], &array[i]);
>      }
> -    free(*host_array);
> +    g_free(*host_array);
>      unlock_user(array, target_addr, 1);
>  
>      return 0;
> @@ -2980,7 +2980,7 @@ static inline abi_long do_msgsnd(int 
> msqid, abi_long msgp,
>  
>      if (!lock_user_struct(VERIFY_READ, target_mb, msgp, 0))
>          return -TARGET_EFAULT;
> -    host_mb = malloc(msgsz+sizeof(long));
> +    host_mb = g_try_malloc(msgsz + sizeof(long));
>      if (!host_mb) {
>          unlock_user_struct(target_mb, msgp, 0);
>          return -TARGET_ENOMEM;
> @@ -2988,7 +2988,7 @@ static inline abi_long do_msgsnd(int 
> msqid, abi_long msgp,
>      host_mb->mtype = (abi_long) tswapal(target_mb->mtype);
>      memcpy(host_mb->mtext, target_mb->mtext, msgsz);
>      ret = get_errno(msgsnd(msqid, host_mb, msgsz, msgflg));
> -    free(host_mb);
> +    g_free(host_mb);
>      unlock_user_struct(target_mb, msgp, 0);
>  
>      return ret;
> @@ -7723,7 +7723,7 @@ abi_long do_syscall(void *cpu_env, int 
> num, abi_long arg1,
>              struct linux_dirent *dirp;
>              abi_long count = arg3;
>  
> -        dirp = malloc(count);
> +        dirp = g_try_malloc(sizeof(count));
>          if (!dirp) {
>                  ret = -TARGET_ENOMEM;
>                  goto fail;
> @@ -7760,7 +7760,7 @@ abi_long do_syscall(void *cpu_env, int 
> num, abi_long arg1,
>          ret = count1;
>                  unlock_user(target_dirp, arg2, ret);
>              }
> -        free(dirp);
> +        g_free(dirp);
>          }
>  #else
>          {
Riku Voipio Oct. 6, 2015, 10:41 a.m. UTC | #3 
On maanantaina 5. lokakuuta 2015 6.32.27 EEST, Harmandeep Kaur wrote:
> Convert malloc()/calloc() calls to g_malloc()/g_try_malloc()/g_new0()
> in linux-user/syscall.c file
>

> @@ -7723,7 +7723,7 @@ abi_long do_syscall(void *cpu_env, int 
> num, abi_long arg1,
>              struct linux_dirent *dirp;
>              abi_long count = arg3;
>  
> -        dirp = malloc(count);
> +        dirp = g_try_malloc(sizeof(count));

Shouldn't this be g_try_malloc(count) ?

>          if (!dirp) {
>                  ret = -TARGET_ENOMEM;
>                  goto fail;
> @@ -7760,7 +7760,7 @@ abi_long do_syscall(void *cpu_env, int 
> num, abi_long arg1,
>          ret = count1;
>                  unlock_user(target_dirp, arg2, ret);
>              }
> -        free(dirp);
> +        g_free(dirp);
>          }
>  #else
>          {
Harmandeep Kaur Oct. 6, 2015, 12:47 p.m. UTC | #4 
On Tue, Oct 6, 2015 at 4:11 PM, Riku Voipio <riku.voipio@iki.fi> wrote:

> On maanantaina 5. lokakuuta 2015 6.32.27 EEST, Harmandeep Kaur wrote:
>
>> Convert malloc()/calloc() calls to g_malloc()/g_try_malloc()/g_new0()
>> in linux-user/syscall.c file
>>
>>
> @@ -7723,7 +7723,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
>> arg1,
>>              struct linux_dirent *dirp;
>>              abi_long count = arg3;
>>  -        dirp = malloc(count);
>> +        dirp = g_try_malloc(sizeof(count));
>>
>
> Shouldn't this be g_try_malloc(count) ?


Yes, I also thinks the same. Thank you for review.

>
>
>          if (!dirp) {
>>                  ret = -TARGET_ENOMEM;
>>                  goto fail;
>> @@ -7760,7 +7760,7 @@ abi_long do_syscall(void *cpu_env, int num,
>> abi_long arg1,
>>          ret = count1;
>>                  unlock_user(target_dirp, arg2, ret);
>>              }
>> -        free(dirp);
>> +        g_free(dirp);
>>          }
>>  #else
>>          {
>>
>
>
diff mbox

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 98b5766..6e90141 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1559,7 +1559,7 @@  set_timeout:
                 }

                 fprog.len = tswap16(tfprog->len);
-                filter = malloc(fprog.len * sizeof(*filter));
+                filter = g_try_new(struct sock_filter, fprog.len);
                 if (filter == NULL) {
                     unlock_user_struct(tfilter, tfprog->filter, 1);
                     unlock_user_struct(tfprog, optval_addr, 1);
@@ -1575,7 +1575,7 @@  set_timeout:

                 ret = get_errno(setsockopt(sockfd, SOL_SOCKET,
                                 SO_ATTACH_FILTER, &fprog, sizeof(fprog)));
-                free(filter);
+                g_free(filter);

                 unlock_user_struct(tfilter, tfprog->filter, 1);
                 unlock_user_struct(tfprog, optval_addr, 1);
@@ -1886,7 +1886,7 @@  static struct iovec *lock_iovec(int type, abi_ulong
target_addr,
         return NULL;
     }

-    vec = calloc(count, sizeof(struct iovec));
+    vec = g_try_new0(struct iovec, count);
     if (vec == NULL) {
         errno = ENOMEM;