On Mon, 7 Mar 2022 10:39:42 +0100 Lukas Bulwahn <lukas.bulwahn@xxxxxxxxx> wrote:
> make clang-analyzer on x86_64 defconfig caught my attention with:
>
> kernel/taskstats.c:120:2: warning: Value stored to 'rc' is never read \
> [clang-analyzer-deadcode.DeadStores]
> rc = 0;
> ^
>
> Commit d94a041519f3 ("taskstats: free skb, avoid returns in
> send_cpu_listeners") made send_cpu_listeners() not return a value and
> hence, the rc variable remained only to be used within the loop where
> it is always assigned before read and it does not need any other
> initialisation.
>
> So, simply remove this unneeded dead initializing assignment.
>
> As compilers will detect this unneeded assignment and optimize this anyway,
> the resulting object code is identical before and after this change.
>
> No functional change. No change to object code.
>
> ...
>
> --- a/kernel/taskstats.c
> +++ b/kernel/taskstats.c
> @@ -117,7 +117,6 @@ static void send_cpu_listeners(struct sk_buff *skb,
>
> genlmsg_end(skb, reply);
>
> - rc = 0;
> down_read(&listeners->sem);
> list_for_each_entry(s, &listeners->list, list) {
> skb_next = NULL;
Yup. It would be better to also reduce the scope of `rc' so later code
can't go and read it uninitialized.
--- a/kernel/taskstats.c~taskstats-remove-unneeded-dead-assignment-fix
+++ a/kernel/taskstats.c
@@ -113,12 +113,14 @@ static void send_cpu_listeners(struct sk
struct listener *s, *tmp;
struct sk_buff *skb_next, *skb_cur = skb;
void *reply = genlmsg_data(genlhdr);
- int rc, delcount = 0;
+ int delcount = 0;
genlmsg_end(skb, reply);
down_read(&listeners->sem);
list_for_each_entry(s, &listeners->list, list) {
+ int rc;
+
skb_next = NULL;
if (!list_is_last(&s->list, &listeners->list)) {
skb_next = skb_clone(skb_cur, GFP_KERNEL);
_
(we could just elimiate `rc' altogether, but I think the above is OK,
perhaps a little more readable).
