Re: [PATCH] percpu: initialize best_upa variable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 5/27/21 1:24 PM, Dennis Zhou wrote:
Hello,

On Mon, May 17, 2021 at 02:39:21PM +0000, Dennis Zhou wrote:
On Mon, May 17, 2021 at 06:17:47AM -0700, Tom Rix wrote:
On 5/16/21 7:05 PM, Dennis Zhou wrote:
Hello,

On Sat, May 15, 2021 at 11:08:17AM -0700, trix@xxxxxxxxxx wrote:
From: Tom Rix <trix@xxxxxxxxxx>

Static analysis reports this problem
percpu.c:2945:6: warning: Assigned value is garbage or undefined
          upa = best_upa;
              ^ ~~~~~~~~
best_upa may not be set, so initialize it.

Signed-off-by: Tom Rix <trix@xxxxxxxxxx>
---
   mm/percpu.c | 1 +
   1 file changed, 1 insertion(+)

diff --git a/mm/percpu.c b/mm/percpu.c
index a257c3efdf18b..6578b706fae81 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -2916,6 +2916,7 @@ static struct pcpu_alloc_info * __init __flatten pcpu_build_alloc_info(
   	 * Related to atom_size, which could be much larger than the unit_size.
   	 */
   	last_allocs = INT_MAX;
+	best_upa = max_upa;
   	for (upa = max_upa; upa; upa--) {
   		int allocs = 0, wasted = 0;
--
2.26.3

I think the proper fix would be:

best_upa = 0;
I was looking for initializing with something that would work.

I think I prefer setting it to 0 because it forces the loop to have
succeeded vs being able to bypass it if the for loop logic was changed.

for (...) { }
BUG_ON(!best_upa);
WARN_ON instead?
This is initialization code. So if upa == 0, it really is a problem.
Having 0 units per allocation is bogus.

upa = best_upa;

If you're fine with this I'll make the changes and apply it to
for-5.13-fixes.

Can you also tell me what static analysis tool produced this? I'm just a
little curious because this code hasn't changed in several years so I'd
have expected some static analyzer to have caught this by now.
Clang 10

Tom

Thanks,
Dennis
Following up here. Are you find with me making the changes and
attributing it to you? Otherwise I can just spin another patch real
quick.

I am fine with you respinning, no need to attribute the change to me.

If you would like a review, include me on the cc.

Thanks!

Tom

At this point I've already sent my PR for-5.13-fixes. So I'll queue some
fix for-5.14.
Thanks,
Dennis






[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux