Messages in this thread | | | Date | Wed, 6 Nov 2013 23:18:07 -0800 | From | Brian Norris <> | Subject | Re: [PATCH -next] mtd: nand: omap: fix error return code in omap_nand_probe() |
| |
On Tue, Nov 05, 2013 at 07:45:27PM -0300, Ezequiel Garcia wrote: > On Tue, Nov 05, 2013 at 01:59:25PM -0800, Brian Norris wrote: > > On Thu, Oct 31, 2013 at 7:18 PM, Jingoo Han <jg1.han@samsung.com> wrote: > > > On Friday, November 01, 2013 9:16 AM, Wei Yongjun wrote: > > >> > > >> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn> > > >> > > >> Fix to return a negative error code from the error handling > > >> case instead of 0, as done elsewhere in this function. > > > > > > Commit message is right? :-( > > > > It sounds OK by my reading. Unless you're having trouble parsing what > > "as done elsewhere in this function" is being applied to. (IOW, is the > > rest of the function returning a negative error code on the error > > paths, or is it returning 0? Of course the answer is the former, but > > it's possible to misread it.) If it helps, I can try to tweak the > > wording a bit when applying this patch. > > > > Pekon, can I get an Acked-by? > > > > I guess you'd prefer Pekon's ack than mine, but anyway:
Eh, this patch was pretty small anyway. But extra eyes are good.
> Acked-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Thanks.
> I'd like to point out this driver has other "mis-behaviors" in returning codes > in some other places. > > In particular, this pattern can be found repeatedly: > > if (do_something()) { > err = -ENXIO; > goto some_other_place; > } > > Which should probably be: > > err = do_something(); > if (err) > goto some_other_place;
Yeah, these could be made more consistent. If the callee is choosing good error codes, then we can just return them. But this is mostly cosmetic.
> Wei: maybe you'd like to prepare some more patches?
Brian
| |