On Tue, 2012-10-02 at 20:11 -0400, valdis.kletni...@vt.edu wrote:
> On Mon, 01 Oct 2012 11:03:21 +0100, Mark Brown said:
> > On Sun, Sep 30, 2012 at 12:15:55PM +0200, Paul Bolle wrote:
> > > Building regmap.o triggers this GCC warning:
> > > drivers/base/regmap/regmap.c: In function regmap_raw_read:
> > > drivers/base/regmap/regmap.c:1172:6: warning: ret may be used
> > > uninitialized in this function [-Wmaybe-uninitialized]
> > >
> > > It seems 'ret' should always be set when this function returns. See, the
> > > else-branch can leave 'ret' uninitialized only if 'val_count' is zero.
> > > But if 'val_count' is zero regmap_volatile_range() will return true.
>
> I've not dug into it that deeply - is there a way that gcc is able to intuit
> this fact and use it for flow analysis? If not, it's not going to be able to
> include that information in its analysis.
I know little about GCC, and even less about the analyses it uses. I
did, however, disassemble regmap_raw_read() with GDB. And the output
this generated does suggest that GCC uses one register to track 'ret'
and that this register is set to -EINVAL quite early. Ie, gdb's
disassembler's output suggests that 'ret' will not be used
uninitialized. (This I need to recheck, though.) But, even assuming I
understood the disassembled code correctly, I have no idea whether we
may expect GCC to understand the code it generates enough to know that,
yes, it did actually set 'ret' itself in that code.
> > > That implies that 'ret' will be set in the if-branch. ('val_count' could
> > > be zero if 'val_len' is, for example, zero. That would be useless input,
> > > however.)
>
> But gcc doesn't know what "useless input" means, semantically.
Correct. When this function is compiled gcc has to take into account
that 'val_len' will be called with useless input, like zero.
By the way, GCC doesn't warn if I add an early check whether 'val_count'
is non-zero:
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index c241ae2..d41527b 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1171,6 +1171,8 @@ int regmap_raw_read(struct regmap *map, unsigned int reg,
void *val,
unsigned int v;
int ret, i;
+ if (!val_count)
+ return -EINVAL;
if (val_len % map->format.val_bytes)
return -EINVAL;
if (reg % map->reg_stride)
That is another way to silence GCC here.
Paul Bolle
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
