[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2][Outreachy Round 12]
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2][Outreachy Round 12] |
Date: |
Sat, 05 Mar 2016 11:42:24 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
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
CHECKPATCH in MAINTAINERS.
Welcome to qemu-devel, Sarah!