[U-Boot] [PATCH 1/2] net/eth.c: make eth_get_dev_by_name(NULL) safe

Detlev Zundel
2011-07-13T11:46:29Z

Hi Helmut,

> On 07/12/2011 11:22 AM, Detlev Zundel wrote:
>
>> > i did go through the level of detail and showed the call graphs ...
>> > none of
>> > which should allow a driver tested as working to even once hit the
>> > NULL path.
>>
>> As I said, these are the call graphs currently existing...
>
> This was also my trail.
>
>> > what i wouldnt mind is annotating the prototype with gcc attributes
>> > saying that the argument is nonnull. ... #define __nonnull(x)
>> > __attribute__((__nonnull__ x)) ... extern struct eth_device
>> > *eth_get_dev_by_name(const char *devname) __nonnull(1); ...
>>
>> This can only catch calls the compiler can statically derive, but
>> still I think it is a good thing.
>>
>
> __nonnull__ is actually a optimization attribute, gcc removes
> tests for NULL in the function body, warnings are only generated if
> one literally writes: eth_get_dev_by_name(NULL), so 'statically
> derive'
> is already exageration.

I just checked and can confirm that currently gcc does not do any static
analysis of char* arguments - however in theory it could.

> This really is no help at all. It would indeed establish a precendence
> to using an IMHO quite flawed attribute in gcc. If I had a vote, I'd
> be against it.

I agree that how this is implemented in gcc is no big help. Rather than
believing documentation I should have checked how this works before
lobbying for it.

> The NDEBUG approach however, as Mike suggested, was what I was
> looking for in the first place.

Great!
Detlev

--
<ESC>:!emacs %
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de