"Fix" var shadowing warnings in macros 41/270441/4
authorMichal Bloch <m.bloch@samsung.com>
Thu, 3 Feb 2022 18:49:55 +0000 (19:49 +0100)
committerHyotaek Shim <hyotaek.shim@samsung.com>
Tue, 8 Feb 2022 04:58:27 +0000 (04:58 +0000)
Introduce some horrible spaghetti code to "fix" var shadowing
in recursive macros which accept code referencing vars declared
inside the macro as arguments.

Change-Id: I426f060630010a0f8a5f1eaa642e2eef6100a26c

tests/kdbus/kdbus-util.h
tests/kdbus/test-activator.c
tests/kdbus/test-policy-priv.c

index ba03333..5775142 100644 (file)
@@ -102,41 +102,52 @@ extern int kdbus_util_verbose;
        if (kdbus_util_verbose) \
                print(X)
 
-#define RUN_FORKED(_child_, _parent_) ASSERT_ZERO(({\
-               pid_t pid, rpid;\
-               int ret;\
+#define RUN_FORKED(_child_, _parent_, _counter_) ASSERT_ZERO(({\
+               pid_t pid##_counter_, rpid##_counter_;\
+               int ret_##_counter_;\
                \
-               pid = fork();\
-               if (pid == 0) {\
+               pid##_counter_ = fork();\
+               if (pid##_counter_ == 0) {\
                        _child_;\
                        exit(0);\
-               } else if (pid > 0) {\
+               } else if (pid##_counter_ > 0) {\
                        _parent_;\
-                       rpid = waitpid(pid, &ret, 0);\
-                       ASSERT_RETURN(rpid,==,pid);\
-                       ASSERT_NONZERO(WIFEXITED(ret));\
-                       ASSERT_ZERO(WEXITSTATUS(ret));\
-                       ret = TEST_OK;\
+                       rpid##_counter_ = waitpid(pid##_counter_, &ret_##_counter_, 0);\
+                       ASSERT_RETURN(rpid##_counter_,==,pid##_counter_);\
+                       ASSERT_NONZERO(WIFEXITED(ret_##_counter_));\
+                       ASSERT_ZERO(WEXITSTATUS(ret_##_counter_));\
+                       ret_##_counter_ = TEST_OK;\
                } else {\
-                       ret = pid;\
+                       ret_##_counter_ = pid##_counter_;\
                }\
                \
-               ret;\
+               ret_##_counter_;\
        }))
 
-#define RUN_UNPRIVILEGED(child_uid, child_gid, _child_, _parent_) RUN_FORKED(({\
-               ret = drop_privileges(child_uid, child_gid);\
-               ASSERT_EXIT_VAL(ret,==,0, ret);\
+#define RUN_UNPRIVILEGED_CTR(child_uid, child_gid, _child_, _parent_, _counter_) RUN_FORKED(({\
+               ret_##_counter_ = drop_privileges(child_uid, child_gid);\
+               ASSERT_EXIT_VAL(ret_##_counter_,==,0, ret_##_counter_);\
                _child_;\
-       }), _parent_)
+       }), _parent_, _counter_)
 
-#define RUN_UNPRIVILEGED_CONN(_var_, _bus_, _code_)                    \
-       RUN_UNPRIVILEGED(UNPRIV_UID, UNPRIV_GID, ({                     \
+/* Looks like a no-op, but expands macros (so that `ret##counter`
+ * in the macro above becomes `ret123` instead of `ret__COUNTER__`). */
+#define RUN_UNPRIVILEGED_EXPAND_MACROS(a,b,c,d,e) \
+       RUN_UNPRIVILEGED_CTR(a,b,c,d,e)
+
+#define RUN_UNPRIVILEGED(child_uid, child_gid, _child_, _parent_) \
+       RUN_UNPRIVILEGED_EXPAND_MACROS(child_uid, child_gid, _child_, _parent_, __COUNTER__)
+
+#define RUN_UNPRIVILEGED_CONN_CTR(_var_, _bus_, _code_, _counter_)                     \
+       RUN_UNPRIVILEGED_EXPAND_MACROS(UNPRIV_UID, UNPRIV_GID, ({                       \
                struct kdbus_conn *_var_;                               \
                ASSERT_EXIT_NONZERO(_var_ = kdbus_hello(_bus_, 0, NULL, 0));                    \
                _code_;                                                 \
                kdbus_conn_free(_var_);                                 \
-       }), ({}))
+       }), ({}), _counter_)
+
+#define RUN_UNPRIVILEGED_CONN(_var_, _bus_, _code_) \
+       RUN_UNPRIVILEGED_CONN_CTR(_var_, _bus_, _code_, __COUNTER__)
 
 #define RUN_CLONE_CHILD(clone_ret, flags, _setup_, _child_body_,       \
                        _parent_setup_, _parent_body_) ({               \
index f2a401c..01cb0c1 100644 (file)
@@ -80,18 +80,34 @@ static wur int kdbus_priv_activator(struct kdbus_test_env *env)
         * Make sure that other users can't TALK to the activator
         */
 
-       RUN_UNPRIVILEGED_CONN(unpriv, env->buspath, ({
-               /* Try to talk using the ID */
-               ret = kdbus_msg_send(unpriv, NULL, 0xdeadbeef, 0, 0,
-                                    0, activator->id);
-               ASSERT_EXIT(ret,==,-ENXIO);
-
-               /* Try to talk to the name */
-               ret = kdbus_msg_send(unpriv, "foo.priv.activator",
-                                    0xdeadbeef, 0, 0, 0,
-                                    KDBUS_DST_ID_NAME);
-               ASSERT_EXIT(ret,==,ONTIZEN(0,-EPERM));
-       }));
+/* This macro nonsense is there because some corporate rule requires people to
+ * compile with -Wshadow, recursive use of the macro requires the use of the
+ * counter not to create shadowing vars, and then the piece of code passed as
+ * the macro param needs to have the same value of the counter everywhere which
+ * necessitates the whole chunk being a macro itself due to expansion rules. */
+#define TEST_CANT_TALK(shadow_counter) \
+\
+       RUN_UNPRIVILEGED_CONN_CTR(unpriv, env->buspath, ({ \
+               /* Try to talk using the ID */ \
+               ret_##shadow_counter = kdbus_msg_send(unpriv, NULL, 0xdeadbeef, 0, 0, \
+                                    0, activator->id); \
+               ASSERT_EXIT(ret_##shadow_counter,==,-ENXIO); \
+\
+               /* Try to talk to the name */ \
+               ret_##shadow_counter = kdbus_msg_send(unpriv, "foo.priv.activator", \
+                                    0xdeadbeef, 0, 0, 0, \
+                                    KDBUS_DST_ID_NAME); \
+               ASSERT_EXIT(ret_##shadow_counter,==,ONTIZEN(0,-EPERM)); \
+       }), shadow_counter)
+
+/* Looks like a no-op, but expands macros (so that `ret##counter`
+ * in the macro above becomes `ret123` instead of `ret__COUNTER__`). */
+#define TEST_CANT_TALK_EXPAND_MACRO(shadow_counter) TEST_CANT_TALK(shadow_counter)
+
+       TEST_CANT_TALK_EXPAND_MACRO(__COUNTER__);
+
+#undef TEST_CANT_TALK_EXPAND_MACRO
+#undef TEST_CANT_TALK
 
        /*
         * Make sure that we did not receive anything, so the
index b7f6044..025cdb0 100644 (file)
@@ -997,32 +997,45 @@ static wur int test_policy_priv(struct kdbus_test_env *env)
 
        ASSERT_ZERO(kdbus_name_acquire(owner, "com.example.c", NULL));
 
-       RUN_UNPRIVILEGED(UNPRIV_UID, UNPRIV_GID, ({
-               struct kdbus_conn *unpriv;
-
-               /* wait for parent to be finished */
-               sigemptyset(&sset);
-               ret = sigsuspend(&sset);
-               ASSERT_RETURN(errno,==,EINTR);
-               ASSERT_RETURN(ret,==,-1);
-
-               ASSERT_NONZERO(unpriv = kdbus_hello(env->buspath, 0, NULL, 0));
-
-               ASSERT_EXIT_ZERO(kdbus_msg_send(unpriv, "com.example.c", 0xdeadbeef, 0, 0, 0, 0));
-
-               ASSERT_EXIT(0,<=,kdbus_msg_recv_poll(owner, 100, NULL, NULL));
-
-               /* free policy holder */
-               kdbus_conn_free(conn);
-
-               ASSERT_EXIT(ONTIZEN(0,-EPERM),==,kdbus_msg_send(unpriv, "com.example.c", 0xdeadbeef, 0, 0, 0, 0));
-
-               kdbus_conn_free(unpriv);
-       }), ({
-               /* make sure policy holder is only valid in child */
-               kdbus_conn_free(conn);
-               kill(pid, SIGUSR1);
-       }));
+/* This macro nonsense is there because some corporate rule requires people to
+ * compile with -Wshadow, recursive use of the macro requires the use of the
+ * counter not to create shadowing vars, and then the piece of code passed as
+ * the macro param needs to have the same value of the counter everywhere which
+ * necessitates the whole chunk being a macro itself due to expansion rules. */
+#define TEST_POLICY_THING(SHADOW_COUNTER) \
+\
+       RUN_UNPRIVILEGED_CTR(UNPRIV_UID, UNPRIV_GID, ({ \
+               struct kdbus_conn *unpriv; \
+\
+               /* wait for parent to be finished */ \
+               sigemptyset(&sset); \
+               ret = sigsuspend(&sset); \
+               ASSERT_RETURN(errno,==,EINTR); \
+               ASSERT_RETURN(ret,==,-1); \
+               ASSERT_NONZERO(unpriv = kdbus_hello(env->buspath, 0, NULL, 0)); \
+               ASSERT_EXIT_ZERO(kdbus_msg_send(unpriv, "com.example.c", 0xdeadbeef, 0, 0, 0, 0)); \
+               ASSERT_EXIT(0,<=,kdbus_msg_recv_poll(owner, 100, NULL, NULL)); \
+\
+               /* free policy holder */ \
+               kdbus_conn_free(conn); \
+\
+               ASSERT_EXIT(ONTIZEN(0,-EPERM),==,kdbus_msg_send(unpriv, "com.example.c", 0xdeadbeef, 0, 0, 0, 0)); \
+\
+               kdbus_conn_free(unpriv); \
+       }), ({ \
+               /* make sure policy holder is only valid in child */ \
+               kdbus_conn_free(conn); \
+               kill(pid##SHADOW_COUNTER, SIGUSR1); \
+       }), SHADOW_COUNTER)
+
+/* Looks like a no-op, but expands macros (so that `pid##counter`
+ * in the macro above becomes `pid123` instead of `pid__COUNTER__`). */
+#define TEST_POLICY_THING_EXPAND_MACRO(SHADOW_COUNTER) TEST_POLICY_THING(SHADOW_COUNTER)
+
+       TEST_POLICY_THING_EXPAND_MACRO(__COUNTER__);
+
+#undef TEST_POLICY_THING
+#undef TEST_POLICY_THING_EXPAND_MACRO
 
        /*
         * The following tests are necessary.