[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2][Outreachy Round 12]

From: Sarah Khan
Subject: Re: [Qemu-devel] [PATCH v2][Outreachy Round 12]
Date: Mon, 7 Mar 2016 11:07:00 +0530

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


On Sat, Mar 5, 2016 at 4:12 PM, Markus Armbruster <address@hidden> 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 <address@hidden> (maintainer:Overall)
    address@hidden (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 <address@hidden> 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 <address@hidden>
> ---
> 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

Welcome to qemu-devel, Sarah!

reply via email to

[Prev in Thread] Current Thread [Next in Thread]