From: Michal Bloch Date: Thu, 3 Feb 2022 18:49:55 +0000 (+0100) Subject: "Fix" var shadowing warnings in macros X-Git-Tag: submit/tizen/20220208.055117~1 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=7a7fc16c7d6576f14bb4470090b3926d02fbba0a;p=platform%2Fkernel%2Flinux-tizen-modules-source.git "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 --- 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.