[Qemu-devel] [PATCH] trivial malloc to g_malloc in thunk

[Qemu-devel] [PATCH] trivial malloc to g_malloc in thunk

[Aarushi Mehta]

Hi

This is a trivial contribution part of the BiteSizedTasks on the wiki.
I found this discussion http://git.corpit.ru/?p=qemu.git;a=commit;h=b45c03f585ea9bb1af76c73e82195418c294919d
on migrating even g_malloc to g_new, is this not appropriate for the same? 
The wiki can presumably use an update regarding this.

Signed-off-by: Aarushi <mehta.aaru20@gmail.com>
---
 thunk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/thunk.c b/thunk.c
index d5d8645cd4..03fb2abab7 100644
--- a/thunk.c
+++ b/thunk.c
@@ -89,7 +89,7 @@ void thunk_register_struct(int id, const char *name, const argtype *types)
     for(i = 0;i < 2; i++) {
         offset = 0;
         max_align = 1;
-        se->field_offsets[i] = malloc(nb_fields * sizeof(int));
+        se->field_offsets[i] = g_malloc(nb_fields * sizeof(int));
         type_ptr = se->field_types;
         for(j = 0;j < nb_fields; j++) {
             size = thunk_type_size(type_ptr, i);
-- 
2.17.1

Re: [Qemu-devel] [PATCH] trivial malloc to g_malloc in thunk

[Daniel P. Berrangé]

On Thu, Feb 28, 2019 at 07:12:45PM +0530, Aarushi Mehta wrote:
> Hi
> 
> This is a trivial contribution part of the BiteSizedTasks on the wiki.
> I found this discussion http://git.corpit.ru/?p=qemu.git;a=commit;h=b45c03f585ea9bb1af76c73e82195418c294919d
> on migrating even g_malloc to g_new, is this not appropriate for the same? 
> The wiki can presumably use an update regarding this.

This kind of question should not be in the commit message - it shoudl
go below the '---'

> 
> Signed-off-by: Aarushi <mehta.aaru20@gmail.com>
> ---

....here....

This lets people answer the question, without the question becoming
part of the git history.

>  thunk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/thunk.c b/thunk.c
> index d5d8645cd4..03fb2abab7 100644
> --- a/thunk.c
> +++ b/thunk.c
> @@ -89,7 +89,7 @@ void thunk_register_struct(int id, const char *name, const argtype *types)
>      for(i = 0;i < 2; i++) {
>          offset = 0;
>          max_align = 1;
> -        se->field_offsets[i] = malloc(nb_fields * sizeof(int));
> +        se->field_offsets[i] = g_malloc(nb_fields * sizeof(int));

Yes, this should use g_new0

>          type_ptr = se->field_types;
>          for(j = 0;j < nb_fields; j++) {
>              size = thunk_type_size(type_ptr, i);
> -- 
> 2.17.1
> 
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] trivial malloc to g_malloc in thunk

[Eric Blake]

On 2/28/19 7:42 AM, Aarushi Mehta wrote:
> Hi
> 
> This is a trivial contribution part of the BiteSizedTasks on the wiki.
> I found this discussion http://git.corpit.ru/?p=qemu.git;a=commit;h=b45c03f585ea9bb1af76c73e82195418c294919d
> on migrating even g_malloc to g_new, is this not appropriate for the same? 
> The wiki can presumably use an update regarding this.
> 
> Signed-off-by: Aarushi <mehta.aaru20@gmail.com>
> ---
>  thunk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/thunk.c b/thunk.c
> index d5d8645cd4..03fb2abab7 100644
> --- a/thunk.c
> +++ b/thunk.c
> @@ -89,7 +89,7 @@ void thunk_register_struct(int id, const char *name, const argtype *types)
>      for(i = 0;i < 2; i++) {
>          offset = 0;
>          max_align = 1;
> -        se->field_offsets[i] = malloc(nb_fields * sizeof(int));
> +        se->field_offsets[i] = g_malloc(nb_fields * sizeof(int));

Where is the counterpart free() that needs to be changed to g_free()?

Also, you absolutely want g_new() or some other variant that separates
the number of elements from the size of the element as two separate
arguments, to avoid the possibility of integer overflow when using * in
a single argument.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] trivial malloc to g_malloc in thunk

[Peter Maydell]

On Thu, 28 Feb 2019 at 14:00, Eric Blake <eblake@redhat.com> wrote:
>
> On 2/28/19 7:42 AM, Aarushi Mehta wrote:
> > Hi
> >
> > This is a trivial contribution part of the BiteSizedTasks on the wiki.
> > I found this discussion http://git.corpit.ru/?p=qemu.git;a=commit;h=b45c03f585ea9bb1af76c73e82195418c294919d
> > on migrating even g_malloc to g_new, is this not appropriate for the same?
> > The wiki can presumably use an update regarding this.
> >
> > Signed-off-by: Aarushi <mehta.aaru20@gmail.com>
> > ---
> >  thunk.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/thunk.c b/thunk.c
> > index d5d8645cd4..03fb2abab7 100644
> > --- a/thunk.c
> > +++ b/thunk.c
> > @@ -89,7 +89,7 @@ void thunk_register_struct(int id, const char *name, const argtype *types)
> >      for(i = 0;i < 2; i++) {
> >          offset = 0;
> >          max_align = 1;
> > -        se->field_offsets[i] = malloc(nb_fields * sizeof(int));
> > +        se->field_offsets[i] = g_malloc(nb_fields * sizeof(int));
>
> Where is the counterpart free() that needs to be changed to g_free()?

There is none, because this code sets up a data structure at
startup which then lasts for the lifetime of the QEMU process.
This is definitely worth noting in the commit message.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] trivial malloc to g_malloc in thunk

[Aarushi Mehta]

On Thu, 2019-02-28 at 13:50 +0000, Daniel P. Berrangé wrote:
> On Thu, Feb 28, 2019 at 07:12:45PM +0530, Aarushi Mehta wrote:
> > Hi
> > 
> > This is a trivial contribution part of the BiteSizedTasks on the
> > wiki.
> > I found this discussion 
> > http://git.corpit.ru/?p=qemu.git;a=commit;h=b45c03f585ea9bb1af76c73e82195418c294919d
> > on migrating even g_malloc to g_new, is this not appropriate for
> > the same? 
> > The wiki can presumably use an update regarding this.
> 
> This kind of question should not be in the commit message - it shoudl
> go below the '---'
> 
> > 
> > Signed-off-by: Aarushi <mehta.aaru20@gmail.com>
> > ---
> 
> ....here....
> 
> This lets people answer the question, without the question becoming
> part of the git history.

Eek, apologies. Thank you.
> >  thunk.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/thunk.c b/thunk.c
> > index d5d8645cd4..03fb2abab7 100644
> > --- a/thunk.c
> > +++ b/thunk.c
> > @@ -89,7 +89,7 @@ void thunk_register_struct(int id, const char
> > *name, const argtype *types)
> >      for(i = 0;i < 2; i++) {
> >          offset = 0;
> >          max_align = 1;
> > -        se->field_offsets[i] = malloc(nb_fields * sizeof(int));
> > +        se->field_offsets[i] = g_malloc(nb_fields * sizeof(int));
> 
> Yes, this should use g_new0

Sent a patch in with the same
> >          type_ptr = se->field_types;
> >          for(j = 0;j < nb_fields; j++) {
> >              size = thunk_type_size(type_ptr, i);
> > -- 
> > 2.17.1
> > 
> > 
> > 
> 
> Regards,
> Daniel