qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: dnbrdsky@gmail.com, stefanha@gmail.com
Cc: "open list:All patches CC here" <qemu-devel@nongnu.org>
Subject: Re: [PATCH] misc: fix __COUNTER__ macro to be referenced properly
Date: Thu, 19 Mar 2020 15:35:42 -0500	[thread overview]
Message-ID: <43c25451-f7f9-41ee-749f-1e73d5b46f9c@redhat.com> (raw)
In-Reply-To: <20200319161925.1818377-1-dnbrdsky@gmail.com>

On 3/19/20 11:19 AM, dnbrdsky@gmail.com wrote:
> From: danbrodsky <dnbrdsky@gmail.com>
> 
> - __COUNTER__ doesn't work with ## concat
> - replaced ## with glue() macro so __COUNTER__ is evaluated
> 
> Signed-off-by: danbrodsky <dnbrdsky@gmail.com>

Thanks - this appears to be your first contribution to qemu.

Typically, the S-o-b should match how you would spell your legal name, 
rather than being a single-word computer user name.

It looks like you threaded another message to this one:
Message-Id: <20200319161925.1818377-2-dnbrdsky@gmail.com>
Subject: [PATCH] lockable: replaced locks with lock guard macros where
  appropriate
but without a 0/2 cover letter, or even a 1/2 or 2/2 indicator on the 
individual patches.  This makes it more likely that the second patch may 
be overlooked by our CI tools.

Since this patch is fixing an issue that just went into the tree 
recently, it would be useful to add mention of that in the commit message:
Fixes: 3284c3ddc4

In fact, using 'lockable:' rather than 'misc:' as your subject prefix 
makes it more obvious that you are fixing an issue in the same area as 
where it was introduced.

More patch submission hints at https://wiki.qemu.org/Contribute/SubmitAPatch

> ---
>   include/qemu/lockable.h | 2 +-
>   include/qemu/rcu.h      | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
> index 1aeb2cb1a6..a9258f2c2c 100644
> --- a/include/qemu/lockable.h
> +++ b/include/qemu/lockable.h
> @@ -170,7 +170,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuLockable, qemu_lockable_auto_unlock)
>    *   }
>    */
>   #define QEMU_LOCK_GUARD(x) \
> -    g_autoptr(QemuLockable) qemu_lockable_auto##__COUNTER__ = \
> +    g_autoptr(QemuLockable) glue(qemu_lockable_auto, __COUNTER__) = \

That said, the patch itself is correct.
Reviewed-by: Eric Blake <eblake@redhat.com>

I'll leave it up to the maintainer for this file whether they can 
improve your commit message (although the hardest part of that would be 
knowing a full proper name to use in place of your username), or if you 
will need to send a v2.

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



  parent reply	other threads:[~2020-03-19 20:36 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-19 16:19 [PATCH] misc: fix __COUNTER__ macro to be referenced properly dnbrdsky
2020-03-19 16:19 ` [PATCH] lockable: replaced locks with lock guard macros where appropriate dnbrdsky
2020-03-19 19:31   ` no-reply
2020-03-19 20:53     ` Eric Blake
2020-03-19 19:39   ` no-reply
2020-03-19 20:50     ` Eric Blake
2020-03-19 19:41   ` no-reply
2020-03-19 20:48   ` Eric Blake
2020-03-19 22:11     ` Daniel Brodsky
2020-03-19 20:35 ` Eric Blake [this message]
2020-03-20 11:09 ` [PATCH] misc: fix __COUNTER__ macro to be referenced properly Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=43c25451-f7f9-41ee-749f-1e73d5b46f9c@redhat.com \
    --to=eblake@redhat.com \
    --cc=dnbrdsky@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).