On Thu, Sep 24, 2015 at 03:47:26PM -0500, atull wrote: > Interesting. The amount of code bloat here compiles down to about two > machine instructions (in two places). Actually a little more since I should > be using IS_ERR_OR_NULL. But the main question is whether I should do > it at all. > They kernel already has too many bogus checks for IS_ERR(). It's a very common bug to check for IS_ERR() when you should be checking for NULL. - foo = some_allocator(); + foo = kmalloc(); if (IS_ERR(foo)) goto fail; I have a static checker for "warn: 'foo' isn't an ERR_PTR" but I haven't published it because too much code has impossible checks. > The behaviour I should drive here is that the user will do their own error > checking. After they get a pointer to a FPGA manager using > of_fpga_mgr_get(), they should check it and not assume that > fpga_mgr_firmware_load() will do it for them, i.e. > > mgr = of_fpga_mgr_get(mgr_node); > if (IS_ERR(mgr)) > return PTR_ERR(mgr); > fpga_mgr_firmware_load(mgr, flags, path); > I don't understand completely how of_fpga_mgr_get() ever returns NULL. A lot of the of_ functions return ERR_PTRs if OF_ is compiled in but they return NULL if it's not. I think this is so people can build with COMPILE_TEST so we get more coverage with static analysis? > I could take out these NULL pointer checks and it won't hurt anything unless > someone is just using the functions badly, in which case: kablooey. Linux devs are very good about doing error checking. An early kablooey is what we want for people who don't. Also if you provide a sanity check then Markus Elfring will remove all the error checking in the callers. Don't do it. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
