Re: [systemd-devel] [PATCH] shared: make container_of() use unique variable names

2014-08-26 Thread Lennart Poettering
On Fri, 22.08.14 14:54, David Herrmann (dh.herrm...@gmail.com) wrote:

Looks good to me. Go ahead, commit.

 If you stack container_of() macros, you will get warnings due to shadowing
 variables of the parent context. To avoid this, use unique names for
 variables.
 
 Two new helpers are added:
   UNIQ: This evaluates to a truly unique value never returned by any
 evaluation of this macro. It's a shortcut for __COUNTER__.
   UNIQ_T: Takes two arguments and concatenates them. It is a shortcut for
   CONCATENATE, but meant to defined typed local variables.
 
 As you usually want to use variables that you just defined, you need to
 reference the same unique value at least two times. However, UNIQ returns
 a new value on each evaluation, therefore, you have to pass the unique
 values into the macro like this:
 
 #define my_macro(a, b) __max_macro(UNIQ, UNIQ, (a), (b))
 #define __my_macro(uniqa, uniqb, a, b) ({
 typeof(a) UNIQ_T(A, uniqa) = (a);
 typeof(b) UNIQ_T(B, uniqb) = (b);
 MY_UNSAFE_MACRO(UNIQ_T(A, uniqa), UNIQ_T(B, uniqb));
 })
 
 This way, MY_UNSAFE_MACRO() can safely evaluate it's arguments multiple
 times as they are local variables. But you can also stack invocations to
 the macro my_macro() without clashing names.
 
 This is the same as if you did:
 
 #define my_macro(a, b) __max_macro(__COUNTER__, __COUNTER__, (a), (b))
 #define __my_macro(prefixa, prefixb, a, b) ({
 typeof(a) CONCATENATE(A, prefixa) = (a);
 typeof(b) CONCATENATE(B, prefixb) = (b);
 MY_UNSAFE_MACRO(CONCATENATE(A, prefixa), CONCATENATE(B, 
 prefixb));
 })
 
 ...but in my opinion, the first macro is easier to write and read.
 
 This patch starts by converting container_of() to use this new helper.
 Other macros may follow (like MIN, MAX, CLAMP, ...).
 ---
  src/shared/macro.h   | 13 -
  src/test/test-util.c | 19 +++
  2 files changed, 27 insertions(+), 5 deletions(-)
 
 diff --git a/src/shared/macro.h b/src/shared/macro.h
 index 2807bc7..e673480 100644
 --- a/src/shared/macro.h
 +++ b/src/shared/macro.h
 @@ -79,6 +79,9 @@
  #define XCONCATENATE(x, y) x ## y
  #define CONCATENATE(x, y) XCONCATENATE(x, y)
  
 +#define UNIQ_T(x, uniq) CONCATENATE(__unique_prefix_, CONCATENATE(x, uniq))
 +#define UNIQ __COUNTER__
 +
  /* Rounds up */
  
  #define ALIGN4(l) (((l) + 3)  ~3)
 @@ -122,13 +125,13 @@ static inline unsigned long ALIGN_POWER2(unsigned long 
 u) {
   * @ptr: the pointer to the member.
   * @type: the type of the container struct this is embedded in.
   * @member: the name of the member within the struct.
 - *
   */
 -#define container_of(ptr, type, member) \
 +#define container_of(ptr, type, member) __container_of(UNIQ, (ptr), type, 
 member)
 +#define __container_of(uniq, ptr, type, member) \
  __extension__ ({\
 -const typeof( ((type *)0)-member ) *__mptr = (ptr); 
 \
 -(type *)( (char *)__mptr - offsetof(type,member) ); \
 -})
 +const typeof( ((type*)0)-member ) *UNIQ_T(A, uniq) = (ptr); 
 \
 +(type*)( (char *)UNIQ_T(A, uniq) - offsetof(type,member) ); \
 +})
  
  #undef MAX
  #define MAX(a,b)\
 diff --git a/src/test/test-util.c b/src/test/test-util.c
 index 34d5f2e..7b2e71c 100644
 --- a/src/test/test-util.c
 +++ b/src/test/test-util.c
 @@ -96,6 +96,24 @@ static void test_max(void) {
  assert_cc(MAXSIZE(char, long) == sizeof(long));
  }
  
 +static void test_container_of(void) {
 +struct mytype {
 +uint8_t pad1[3];
 +uint64_t v1;
 +uint8_t pad2[2];
 +uint32_t v2;
 +} _packed_ myval = { };
 +
 +assert_cc(sizeof(myval) == 17);
 +assert_se(container_of(myval.v1, struct mytype, v1) == myval);
 +assert_se(container_of(myval.v2, struct mytype, v2) == myval);
 +assert_se(container_of(container_of(myval.v2,
 + struct mytype,
 + v2)-v1,
 +   struct mytype,
 +   v1) == myval);
 +}
 +
  static void test_first_word(void) {
  assert_se(first_word(Hello, ));
  assert_se(first_word(Hello, Hello));
 @@ -1218,6 +1236,7 @@ int main(int argc, char *argv[]) {
  test_streq_ptr();
  test_align_power2();
  test_max();
 +test_container_of();
  test_first_word();
  test_close_many();
  test_parse_boolean();


Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org

[systemd-devel] [PATCH] shared: make container_of() use unique variable names

2014-08-22 Thread David Herrmann
If you stack container_of() macros, you will get warnings due to shadowing
variables of the parent context. To avoid this, use unique names for
variables.

Two new helpers are added:
  UNIQ: This evaluates to a truly unique value never returned by any
evaluation of this macro. It's a shortcut for __COUNTER__.
  UNIQ_T: Takes two arguments and concatenates them. It is a shortcut for
  CONCATENATE, but meant to defined typed local variables.

As you usually want to use variables that you just defined, you need to
reference the same unique value at least two times. However, UNIQ returns
a new value on each evaluation, therefore, you have to pass the unique
values into the macro like this:

#define my_macro(a, b) __max_macro(UNIQ, UNIQ, (a), (b))
#define __my_macro(uniqa, uniqb, a, b) ({
typeof(a) UNIQ_T(A, uniqa) = (a);
typeof(b) UNIQ_T(B, uniqb) = (b);
MY_UNSAFE_MACRO(UNIQ_T(A, uniqa), UNIQ_T(B, uniqb));
})

This way, MY_UNSAFE_MACRO() can safely evaluate it's arguments multiple
times as they are local variables. But you can also stack invocations to
the macro my_macro() without clashing names.

This is the same as if you did:

#define my_macro(a, b) __max_macro(__COUNTER__, __COUNTER__, (a), (b))
#define __my_macro(prefixa, prefixb, a, b) ({
typeof(a) CONCATENATE(A, prefixa) = (a);
typeof(b) CONCATENATE(B, prefixb) = (b);
MY_UNSAFE_MACRO(CONCATENATE(A, prefixa), CONCATENATE(B, 
prefixb));
})

...but in my opinion, the first macro is easier to write and read.

This patch starts by converting container_of() to use this new helper.
Other macros may follow (like MIN, MAX, CLAMP, ...).
---
 src/shared/macro.h   | 13 -
 src/test/test-util.c | 19 +++
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/shared/macro.h b/src/shared/macro.h
index 2807bc7..e673480 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -79,6 +79,9 @@
 #define XCONCATENATE(x, y) x ## y
 #define CONCATENATE(x, y) XCONCATENATE(x, y)
 
+#define UNIQ_T(x, uniq) CONCATENATE(__unique_prefix_, CONCATENATE(x, uniq))
+#define UNIQ __COUNTER__
+
 /* Rounds up */
 
 #define ALIGN4(l) (((l) + 3)  ~3)
@@ -122,13 +125,13 @@ static inline unsigned long ALIGN_POWER2(unsigned long u) 
{
  * @ptr: the pointer to the member.
  * @type: the type of the container struct this is embedded in.
  * @member: the name of the member within the struct.
- *
  */
-#define container_of(ptr, type, member) \
+#define container_of(ptr, type, member) __container_of(UNIQ, (ptr), type, 
member)
+#define __container_of(uniq, ptr, type, member) \
 __extension__ ({\
-const typeof( ((type *)0)-member ) *__mptr = (ptr); \
-(type *)( (char *)__mptr - offsetof(type,member) ); \
-})
+const typeof( ((type*)0)-member ) *UNIQ_T(A, uniq) = (ptr); \
+(type*)( (char *)UNIQ_T(A, uniq) - offsetof(type,member) ); \
+})
 
 #undef MAX
 #define MAX(a,b)\
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 34d5f2e..7b2e71c 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -96,6 +96,24 @@ static void test_max(void) {
 assert_cc(MAXSIZE(char, long) == sizeof(long));
 }
 
+static void test_container_of(void) {
+struct mytype {
+uint8_t pad1[3];
+uint64_t v1;
+uint8_t pad2[2];
+uint32_t v2;
+} _packed_ myval = { };
+
+assert_cc(sizeof(myval) == 17);
+assert_se(container_of(myval.v1, struct mytype, v1) == myval);
+assert_se(container_of(myval.v2, struct mytype, v2) == myval);
+assert_se(container_of(container_of(myval.v2,
+ struct mytype,
+ v2)-v1,
+   struct mytype,
+   v1) == myval);
+}
+
 static void test_first_word(void) {
 assert_se(first_word(Hello, ));
 assert_se(first_word(Hello, Hello));
@@ -1218,6 +1236,7 @@ int main(int argc, char *argv[]) {
 test_streq_ptr();
 test_align_power2();
 test_max();
+test_container_of();
 test_first_word();
 test_close_many();
 test_parse_boolean();
-- 
2.1.0

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel