diff mbox series

misc: fix __COUNTER__ macro to be referenced properly

Message ID 20200319161925.1818377-1-dnbrdsky@gmail.com
State New
Headers show
Series misc: fix __COUNTER__ macro to be referenced properly | expand

Commit Message

Daniel Brodsky March 19, 2020, 4:19 p.m. UTC
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>
---
 include/qemu/lockable.h | 2 +-
 include/qemu/rcu.h      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Eric Blake March 19, 2020, 8:35 p.m. UTC | #1
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.
Stefan Hajnoczi March 20, 2020, 11:09 a.m. UTC | #2
On Thu, Mar 19, 2020 at 09:19:24AM -0700, 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>
> ---
>  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__) = \
>              qemu_lockable_auto_lock(QEMU_MAKE_LOCKABLE((x)))
>  
>  #endif

Please fix WITH_QEMU_LOCK_GUARD() too.  It's in the same header file and
gcc -E shows that it also fails to expand __COUNTER__:

  for (__attribute__((cleanup(glib_autoptr_cleanup_QemuLockable)))
      QemuLockable_autoptr qemu_lockable_auto__COUNTER__ = ...

Thanks,
Stefan
diff mbox series

Patch

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__) = \
             qemu_lockable_auto_lock(QEMU_MAKE_LOCKABLE((x)))
 
 #endif
diff --git a/include/qemu/rcu.h b/include/qemu/rcu.h
index 9c82683e37..570aa603eb 100644
--- a/include/qemu/rcu.h
+++ b/include/qemu/rcu.h
@@ -170,7 +170,7 @@  static inline void rcu_read_auto_unlock(RCUReadAuto *r)
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(RCUReadAuto, rcu_read_auto_unlock)
 
 #define WITH_RCU_READ_LOCK_GUARD() \
-    WITH_RCU_READ_LOCK_GUARD_(_rcu_read_auto##__COUNTER__)
+    WITH_RCU_READ_LOCK_GUARD_(glue(_rcu_read_auto, __COUNTER__))
 
 #define WITH_RCU_READ_LOCK_GUARD_(var) \
     for (g_autoptr(RCUReadAuto) var = rcu_read_auto_lock(); \