Hi Peter,

Can you please tell me if I should use other glib
functions, or implement it in any other way?
I am newbie here :)

Thank you for reviewing my patch.

On Thu, Oct 1, 2015 at 9:33 PM, Peter Maydell <[email protected]>
wrote:

> On 1 October 2015 at 06:32, Harmandeep Kaur <[email protected]>
> wrote:
> > Convert malloc()/calloc() calls to g_malloc()/g_try_malloc()/g_new0()
> > in linux-user/syscall.c file
> >
> > 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))
> >
> > Signed-off-by: Harmandeep Kaur <[email protected]>
> > ---
> >  linux-user/syscall.c | 38 +++++++++++---------------------------
> >  1 file changed, 11 insertions(+), 27 deletions(-)
> >
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index d1d3eb2..c79e862 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -1554,12 +1554,7 @@ set_timeout:
> >                  }
> >
> >                  fprog.len = tswap16(tfprog->len);
> > -                filter = malloc(fprog.len * sizeof(*filter));
> > -                if (filter == NULL) {
> > -                    unlock_user_struct(tfilter, tfprog->filter, 1);
> > -                    unlock_user_struct(tfprog, optval_addr, 1);
> > -                    return -TARGET_ENOMEM;
> > -                }
> >
> > +                filter = g_malloc(fprog.len * sizeof(*filter));
>
> fprog.len comes from the guest -- you can't use g_malloc.
>
> >                  for (i = 0; i < fprog.len; i++) {
> >                      filter[i].code = tswap16(tfilter[i].code);
> >                      filter[i].jt = tfilter[i].jt;
> > @@ -1570,7 +1565,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);
> > @@ -1881,11 +1876,7 @@ static struct iovec *lock_iovec(int type,
> abi_ulong
> > target_addr,
> >          return NULL;
> >      }
> >
> > -    vec = calloc(count, sizeof(struct iovec));
> > -    if (vec == NULL) {
> > -        errno = ENOMEM;
> > -        return NULL;
> > -    }
> > +    vec = g_new0(struct iovec, count);
>
> count comes from the guest -- you can't use g_new0.
>
> >
> >      target_vec = lock_user(VERIFY_READ, target_addr,
> >                             count * sizeof(struct target_iovec), 1);
> > @@ -1945,7 +1936,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;
> >  }
> > @@ -2672,14 +2663,11 @@ 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));
> > -    if (!*host_array) {
> > -        return -TARGET_ENOMEM;
> > -    }
> > +    *host_array = g_malloc(nsems*sizeof(unsigned short));
> >      array = lock_user(VERIFY_READ, target_addr,
> >                        nsems*sizeof(unsigned short), 1);
>
> I don't think we can guarantee that nsems is small -- you can't
> use g_malloc.
>
> >      if (!array) {
> > -        free(*host_array);
> > +        g_free(*host_array);
> >          return -TARGET_EFAULT;
> >      }
> >
> > @@ -2716,7 +2704,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;
> > @@ -2975,15 +2963,11 @@ 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));
> > -    if (!host_mb) {
> > -        unlock_user_struct(target_mb, msgp, 0);
> > -        return -TARGET_ENOMEM;
> > -    }
> > +    host_mb = g_malloc(msgsz+sizeof(long));
>
> msgsz comes from the guest -- you can't use g_malloc.
>
> >      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;
> > @@ -7625,7 +7609,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(count);
> >          if (!dirp) {
> >                  ret = -TARGET_ENOMEM;
> >                  goto fail;
> > @@ -7662,7 +7646,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
> >          {
> > --
> > 1.9.1
>
> thanks
> -- PMM
>

Reply via email to