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

[Helmut Raiger <helmut.raiger@hale.at>]

On 07/07/2011 07:46 PM, Mike Frysinger wrote:
>
> those NULL checks should not be necessary either.  a correctly written
> networking driver should only register itself with the miiphy layer
> when it has successfully registered itself with the eth layer.  thus
> any of the miiphy callbacks should always come in with a name that is
> found via eth_get_dev_by_name().
>
You are right, in a perfect world nobody ever falters.

> checking for NULLs here and gracefully returning is unnecessary
> overhead imo as you're only catering to broken code.  fix the broken
> drivers instead.

I could not find drivers that have the potential of delivering NULLs to 
the miiphy_read and write functions, I simply refused to test at  this 
level. Either there is a potential of having NULL passed then the test 
should be in eth_get_dev_by_name() or there is none then we should 
simply leave it away.
I'm rather indifferent to either solution.
And about 'catering to broken code': It must be broken in 2 ways, first 
it must pass a NULL to the function and second it must ignore the return 
code.

> by your logic, why put the NULL check in eth_get_dev_by_name() ?  why
> not handle strcmp(NULL, NULL) too ?  then eth_get_dev_by_name() would
> automatically get "fixed" as would all other call points.
>
For strcmp() you have several issues at hand: Compatibility, performance 
and even a logical problem. What should be the result in case one of the 
arguments is NULL (or both). The logic for eth_get_dev_by_name() is 
completely sane "There is no ethernet device named (NULL)", performance 
and compatibility don't matter. Your comparison is flawed.

And finally we are discussing a few _chararacters_ of code and a 
probably a single assembly instruction.

Helmut


--
Scanned by MailScanner.