Message ID | CAPtdW17fGpP_DUMJWddv+W4ThfC96RVSb2QDRg8TdypEbvmQdw@mail.gmail.com |
---|---|
State | New |
Headers | show |
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>
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 > {
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 > {
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 --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;
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 {