From 7a7fc16c7d6576f14bb4470090b3926d02fbba0a Mon Sep 17 00:00:00 2001 From: Michal Bloch Date: Thu, 3 Feb 2022 19:49:55 +0100 Subject: [PATCH] "Fix" var shadowing warnings in macros 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 | 51 +++++++++++++++----------- tests/kdbus/test-activator.c | 40 ++++++++++++++------- tests/kdbus/test-policy-priv.c | 65 ++++++++++++++++++++-------------- 3 files changed, 98 insertions(+), 58 deletions(-) diff --git a/tests/kdbus/kdbus-util.h b/tests/kdbus/kdbus-util.h index ba03333..5775142 100644 --- a/tests/kdbus/kdbus-util.h +++ b/tests/kdbus/kdbus-util.h @@ -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_) ({ \ diff --git a/tests/kdbus/test-activator.c b/tests/kdbus/test-activator.c index f2a401c..01cb0c1 100644 --- a/tests/kdbus/test-activator.c +++ b/tests/kdbus/test-activator.c @@ -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 diff --git a/tests/kdbus/test-policy-priv.c b/tests/kdbus/test-policy-priv.c index b7f6044..025cdb0 100644 --- a/tests/kdbus/test-policy-priv.c +++ b/tests/kdbus/test-policy-priv.c @@ -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. -- 2.34.1