From mboxrd@z Thu Jan 1 00:00:00 1970 From: Helmut Raiger Date: Mon, 11 Jul 2011 11:53:49 +0200 Subject: [U-Boot] [PATCH 1/2] net/eth.c: make eth_get_dev_by_name(NULL) safe In-Reply-To: References: <4E118AD9.9000200@hale.at> <201107042344.55693.vapier@gentoo.org> <4E140B7C.7090802@hale.at> <201107061538.45781.vapier@gentoo.org> <4E154E34.3090800@hale.at> Message-ID: <4E1AC82D.3020205@hale.at> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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.