linux-kernel - Re: [PATCH] locking/lockdep: Remove unused variable in __lockdep

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <728c68cc-fef0-c48d-056b-6ed7352b443a@redhat.com>
Date:   Sun, 30 Oct 2022 11:20:59 -0400
From:   Waiman Long <longman@...hat.com>
To:     wuchi <wuchi.zero@...il.com>, peterz@...radead.org,
        mingo@...hat.com, will@...nel.org, boqun.feng@...il.com
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH] locking/lockdep: Remove unused variable in
 __lockdep_count*()

On 10/30/22 07:17, wuchi wrote:
> The target_entry variable will never be referenced because the
> function noop_count() always returns false in __bfs().
> So just remove that.
>
> Signed-off-by: wuchi <wuchi.zero@...il.com>
> ---
>   kernel/locking/lockdep.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index e3375bc40dad..04d2ce5d0215 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -2075,9 +2075,8 @@ static bool noop_count(struct lock_list *entry, void *data)
>   static unsigned long __lockdep_count_forward_deps(struct lock_list *this)
>   {
>   	unsigned long  count = 0;
> -	struct lock_list *target_entry;
>   
> -	__bfs_forwards(this, (void *)&count, noop_count, NULL, &target_entry);
> +	__bfs_forwards(this, (void *)&count, noop_count, NULL, NULL);
>   
>   	return count;
>   }
> @@ -2100,9 +2099,8 @@ unsigned long lockdep_count_forward_deps(struct lock_class *class)
>   static unsigned long __lockdep_count_backward_deps(struct lock_list *this)
>   {
>   	unsigned long  count = 0;
> -	struct lock_list *target_entry;
>   
> -	__bfs_backwards(this, (void *)&count, noop_count, NULL, &target_entry);
> +	__bfs_backwards(this, (void *)&count, noop_count, NULL, NULL);
>   
>   	return count;
>   }

There is no target_entry NULL check in __bfs(), so target_entry is 
always expected to point to a valid memory location. You will need to 
add the NULL check if you want to remove it from these two functions.

BTW, have you actually exercised the code to make sure that there was no 
unexpected side effect from this change?

Cheers,
Longman