Re: [PATCH] regmap: Fix compile warning with value uninitialized

From: Mark Brown
Date: Sat Nov 09 2013 - 13:58:39 EST


On Sat, Nov 09, 2013 at 04:34:07PM +0100, Levente Kurusa wrote:
> 2013-11-09 16:16 keltezéssel, Mark Brown írta:

> >> - int i, ret;
> >> + int i;
> >> + int ret = 0;
> >> bool bypass;

> Wouldn't the following be better?

> int i, ret = 0;

> I think it is more readable.

No, that's not the kernel coding style.

> > This sort of fix isn't ideal, it just silences the warning but if the
> > compiler has spotted a genuine oversight in the function it won't
> > address it. It's better to include some analysis of why this is a good
> > fix.

> The only condition when 'ret' is not set is when the num_regs parameter is zero
> and krealloc doesn't fail. If the above two conditions apply, then
> the code would return an uninitialized value. However, calling this function with
> num_regs == 0, would be a waste as it essentially does nothing.

OK, so that should be in the changelog - or there should be an error
check for num_regs == 0 which might be more helpful.

> Also, I think code which throws warnings is worse than code
> which doesn't.

Right, but code that is behaves incorrectly is worse still. We need to
be careful that when changing the code to remove warnings we make sure
that we fix any real problems the compiler was warning us about.

Attachment: signature.asc
Description: Digital signature