On Tue, Jan 18, 2011 at 14:07, Bing Zhao <bzhao@xxxxxxxxxxx> wrote: > Hi Julian, > >> -----Original Message----- >> From: Julian Calaby [mailto:julian.calaby@xxxxxxxxx] >> Sent: Friday, January 14, 2011 6:59 PM >> To: Bing Zhao >> Cc: linux-wireless@xxxxxxxxxxxxxxx; John W. Linville; Johannes Berg; Amitkumar Karwar; Kiran Divekar; >> Frank Huang >> Subject: Re: [PATCH] mwifiex: remove duplicated NULL checks >> >> On Sat, Jan 15, 2011 at 06:36, Bing Zhao <bzhao@xxxxxxxxxxx> wrote: >> > From: Amitkumar Karwar <akarwar@xxxxxxxxxxx> >> > >> > Some functions have unnecessory NULL check for an input parameter >> > because the caller to those functions is already checking the parameter >> > for NULL before calling them. >> >> It may be more readable and efficient to eliminate the NULL checks in >> the callers - and adjust the code to inform the callers of the case >> where the pointer passed is null and make the callers to properly >> handle this case. > > Thank you very much for your comments. > > There are still some cases that the NULL check needs to be done at the first place. For example, > [snip] In this case, dropping the null check is the right thing to do, given that you can guarantee that all callers of mwiflex_init_cmd_node() will check that the pointer isn't null before calling it. >> This should also make the driver more robust as failures can be passed >> up the stack to places that can deal with them more effectively than >> just printing a message. > > I could have misunderstood your point, however. If so, could you please explain a little bit more on how you would make the changes based on above example? I was referring more to that fact that functions that appear to do things, e.g. mwifiex_init_cmd_node(), don't return anything - hence if something fails within them, e.g. being passed a null pointer, they can't notify their caller, e.g. mwifiex_prepare_cmd() that something has gone wrong. That said, I haven't looked at the code except through your patches, so I can't say if this is a scenario that actually could happen, and whether this added functionality is worthwhile. Thanks, -- Julian Calaby Email: julian.calaby@xxxxxxxxx Profile: http://www.google.com/profiles/julian.calaby/ .Plan: http://sites.google.com/site/juliancalaby/ -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
