Re: [PATCH v11 3/4] add FPGA manager core

From: Pavel Machek
Date: Thu Sep 24 2015 - 17:13:40 EST


> > Of course, the maintainer gets the last word regardless of what anyone
> > else thinks.
> >
> > Generally, minimal code is better. Trying to future proof code is a
> > waste of time because you can't predict what will happen in the future.
> > It's way more likely that some pointer you never expected to be NULL
> > will be NULL instead of the few checked at the beginning of a function.
> > Adding useless code uses RAM and makes the function slower. It's a bit
> > confusing for users as well because they will wonder when the NULL check
> > is used. A lot of times this sort of error handling is a bit fake and
> > what I mean is that it looks correct but the system will just crash in a
> > later function.
> >
> > Also especially with a simple NULL dereferences like this theoretical
> > one, it's better to just get the oops. It kills the module but you get
> > a good message in the log and it's normally straight forward to debug.
> >
> > We spent a surprising amount of time discussing useless code. I made
> > someone redo a patch yesterday because they had incomplete error
> > handling for a situation which could never happen.
> Thanks for the discussion.
> 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.
> 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 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.

2 instructions is not that bad, do whatever is easier for you. These
patches received enough bikeshed painting.

Best regards,

(cesky, pictures)
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at