On Mon, 1 Dec 2014, Dan Carpenter wrote:
> On Sat, Nov 29, 2014 at 10:59:47PM +0900, OGAWA Hirofumi wrote:
> > Julia Lawall <julia.lawall@xxxxxxx> writes:
> >
> > >> iput() checks NULL of inode. What is wrong just remove NULL check,
> > >> instead of adding new jump labels?
> > >
> > > Personally, I prefer that code that can be statically determined not to
> > > need to be executed not to be executed. It can make the code easier to
> > > understand, because each function is only called when doing so is useful,
> > > and it can be helpful to static analysis.
> >
> > Hm, first of all, we want to prevent the bugs. More labels are more
> > chances of bug (and we don't care micro optimize on this error path),
> > isn't it? Increasing the chance of bugs and bothers developers for
> > analyzer sounds like strange.
>
> Oh wow! Absolutely not. "One Err Bugs" are one of the most common
> kinds of bugs we have in the kernel. This is where you have just one
> error label and the bugs look like this:
>
> err:
> kfree(foo->bar);
> kfree(foo);
>
> but foo is NULL. Mixing the error paths together it always creates
> confusion. I fix so many of these bugs... We get a few new ones every
> week.
Just for concreteness, from drivers/clk/st/clkgen-mux.c (- indicates
lines of interest, not lines to remove):
@@ -722,7 +722,6 @@ void __init st_of_clkgen_vcc_setup(struc
return;
clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
- if (!clk_data)
goto err;
clk_data->clk_num = VCC_MAX_CHANNELS;
@@ -808,7 +807,6 @@ void __init st_of_clkgen_vcc_setup(struc
return;
err:
- for (i = 0; i < clk_data->clk_num; i++) {
struct clk_composite *composite;
if (!clk_data->clks[i])
julia
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
