[v2,Outreachy,Round,12]

Replaced g_malloc() with g_new() as it would detect multiplication overflow if nb_fields ever oversize.

There is no corresponding free(). thunk_register_struct() is called
only at startup from the linux-user code in order to populate the
struct_entries array; this data structure then remains live for
the entire lifetime of the program and is automatically freed when
QEMU exits.

This replacement was suggested as part of the bite-sized tasks.

Signed-off-by: Sarah Khan <sarahjmi07@gmail.com>
---
Changes in v2 :Make commit message clearer
	       Replace g_malloc() with g_new()	
---
 thunk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Your commit message isn't quite right, yet.  The first line is empty,
which leads to

    Subject: [PATCH v2][Outreachy Round 12]

in e-mail, which isn't really useful.  It should be a line of the form

    subsystem: Headline describing the patch briefly

In e-mail, this becomes something like

    Subject: [PATCH v2] subsystem: Headline describing the patch briefly

The [PATCH v2] gets inserted by git-format-patch.

Finding the appropriate subsystem is unfortunately less than
straightforward.  You can use "git log --oneline FILENAME..." for ideas.
For your patch, I'd use linux-user.

Since this is a rather busy list, it's important to cc the right people
on patches.  Start with "scripts/get_maintainer PATCH-FILE".  The script
looks up the files you patch in MAINTAINERS for you.  In your case, its
output is

    Riku Voipio <riku.voipio@iki.fi> (maintainer:Overall)
    qemu-devel@nongnu.org (open list:All patches CC here)

Send the patch to qemu-devel, cc: Riku.

All of this and much more is documented at
<http://wiki.qemu.org/Contribute/SubmitAPatch>.  It's a learning curve,
but we'll gladly help you along.

Sarah Khan <sarahjmi07@gmail.com> writes:

> Replaced g_malloc() with g_new() as it would detect multiplication overflow if nb_fields ever oversize.

Long line, please break it like you do in the next paragraph.

> There is no corresponding free(). thunk_register_struct() is called
> only at startup from the linux-user code in order to populate the
> struct_entries array; this data structure then remains live for
> the entire lifetime of the program and is automatically freed when
> QEMU exits.
>
> This replacement was suggested as part of the bite-sized tasks.
>
> Signed-off-by: Sarah Khan <sarahjmi07@gmail.com>
> ---
> Changes in v2 :Make commit message clearer
> 	       Replace g_malloc() with g_new()	
> ---
>  thunk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/thunk.c b/thunk.c
> index bddabae..6237702 100644
> --- a/thunk.c
> +++ b/thunk.c
> @@ -88,7 +88,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] = g_malloc(nb_fields * sizeof(int));
> +        se->field_offsets[i] = g_new(int,nb_fields);
>          type_ptr = se->field_types;
>          for(j = 0;j < nb_fields; j++) {
>              size = thunk_type_size(type_ptr, i);

Oh, this is an *incremental* patch: it applies on top of your v1.
That's awkward.  Your v2 should *replace* v1, not add to it.

Style nitpick: we want a space after comma.  Recommend to check your
patches with scripts/checkpatch.pl.  Output for this one is:

    ERROR: space required after that ',' (ctx:VxV)
    #127: FILE: thunk.c:91:
    +        se->field_offsets[i] = g_new(int,nb_fields);
                                             ^

    total: 1 errors, 0 warnings, 8 lines checked

    /home/armbru/Mail/mail/redhat/xlst/qemu-devel/385421 has style problems, please review.  If any of these errors
    are false positives report them to the maintainer, see
    CHECKPATCH in MAINTAINERS.

Welcome to qemu-devel, Sarah!

Thank you for your feedback.
Will do the required.
Sorry for the delay as I was out of station.


Thanks,
Sarah

On Sat, Mar 5, 2016 at 4:12 PM, Markus Armbruster <armbru@redhat.com> wrote:

> Your commit message isn't quite right, yet.  The first line is empty,
> which leads to
>
>     Subject: [PATCH v2][Outreachy Round 12]
>
> in e-mail, which isn't really useful.  It should be a line of the form
>
>     subsystem: Headline describing the patch briefly
>
> In e-mail, this becomes something like
>
>     Subject: [PATCH v2] subsystem: Headline describing the patch briefly
>
> The [PATCH v2] gets inserted by git-format-patch.
>
> Finding the appropriate subsystem is unfortunately less than
> straightforward.  You can use "git log --oneline FILENAME..." for ideas.
> For your patch, I'd use linux-user.
>
> Since this is a rather busy list, it's important to cc the right people
> on patches.  Start with "scripts/get_maintainer PATCH-FILE".  The script
> looks up the files you patch in MAINTAINERS for you.  In your case, its
> output is
>
>     Riku Voipio <riku.voipio@iki.fi> (maintainer:Overall)
>     qemu-devel@nongnu.org (open list:All patches CC here)
>
> Send the patch to qemu-devel, cc: Riku.
>
> All of this and much more is documented at
> <http://wiki.qemu.org/Contribute/SubmitAPatch>.  It's a learning curve,
> but we'll gladly help you along.
>
> Sarah Khan <sarahjmi07@gmail.com> writes:
>
> > Replaced g_malloc() with g_new() as it would detect multiplication
> overflow if nb_fields ever oversize.
>
> Long line, please break it like you do in the next paragraph.
>
> > There is no corresponding free(). thunk_register_struct() is called
> > only at startup from the linux-user code in order to populate the
> > struct_entries array; this data structure then remains live for
> > the entire lifetime of the program and is automatically freed when
> > QEMU exits.
> >
> > This replacement was suggested as part of the bite-sized tasks.
> >
> > Signed-off-by: Sarah Khan <sarahjmi07@gmail.com>
> > ---
> > Changes in v2 :Make commit message clearer
> >              Replace g_malloc() with g_new()
> > ---
> >  thunk.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/thunk.c b/thunk.c
> > index bddabae..6237702 100644
> > --- a/thunk.c
> > +++ b/thunk.c
> > @@ -88,7 +88,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] = g_malloc(nb_fields * sizeof(int));
> > +        se->field_offsets[i] = g_new(int,nb_fields);
> >          type_ptr = se->field_types;
> >          for(j = 0;j < nb_fields; j++) {
> >              size = thunk_type_size(type_ptr, i);
>
> Oh, this is an *incremental* patch: it applies on top of your v1.
> That's awkward.  Your v2 should *replace* v1, not add to it.
>
> Style nitpick: we want a space after comma.  Recommend to check your
> patches with scripts/checkpatch.pl.  Output for this one is:
>
>     ERROR: space required after that ',' (ctx:VxV)
>     #127: FILE: thunk.c:91:
>     +        se->field_offsets[i] = g_new(int,nb_fields);
>                                              ^
>
>     total: 1 errors, 0 warnings, 8 lines checked
>
>     /home/armbru/Mail/mail/redhat/xlst/qemu-devel/385421 has style
> problems, please review.  If any of these errors
>     are false positives report them to the maintainer, see
>     CHECKPATCH in MAINTAINERS.
>
> Welcome to qemu-devel, Sarah!
>