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

Helmut Raiger
2011-07-11T09:53:49Z

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.