On Saturday, November 19, 2016 12:56:57 PM CET Mauro Carvalho Chehab wrote:
> As Arnd reported:
>
> With gcc-5 or higher on x86, we can get a bogus warning in the
> dvb-net code:
>
> drivers/media/dvb-core/dvb_net.c: In function ‘dvb_net_ule’:
> arch/x86/include/asm/string_32.h:77:14: error: ‘dest_addr’ may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/media/dvb-core/dvb_net.c:633:8: note: ‘dest_addr’ was declared
> here
>
> Inspecting the code is really hard, as the function Arnd patched is really
> complex.
>
> IMHO, the best is to first simplify the logic, by breaking parts of it into
> sub-routines, and then apply a proper fix.
>
> This patch series does that.
>
> Arnd,
>
> After splitting the function, I think that the GCC 5 warning is not bogus,
> as this code:
> skb_copy_from_linear_data(h->priv->ule_skb, dest_addr,
> ETH_ALEN);
>
> is called before initializing dest_dir, but, even if I'm wrong, it is not a
> bad
> idea to zero the dest_addr before handing the logic.
My conclusion after looking at it for a while was that it is correct, the
relevant code is roughtly:
if (!priv->ule_dbit) {
drop = ...;
if (drop)
goto done;
else
skb_copy_from_linear_data(priv->ule_skb, dest_addr,
ETH_ALEN);
}
...
if (!priv->ule_dbit) {
memcpy(ethh->h_dest, dest_addr, ETH_ALEN)
}
done:
...
So it is always copied from the skb data before it gets used.
> PS.: I took a lot of care to avoid breaking something on this series, as I
> don't
> have any means here to test DVB net. So, I'd appreciate if you could take
> a look and see if everything looks fine.
I have replaced my patch with your series in my randconfig builds and see no
new warnings so far.
The first patch looks correct to me, but I can't really verify the
second one by inspection. It looks like a nice cleanup and I'd assume
you did it correctly too. The third patch is probably not needed now,
I think with the 'goto' removed, gcc will be able to figure it out
already. It probably adds a few extra cycles to copy the zero data,
which shouldn't be too bad either.
Arnd
[btw, your mche...@s-opensource.com keeps bouncing for me, I had to
remove that from Cc to get my reply to make it out]
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
