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.