Re: [PATCH v2] fs-fat: Less function calls in fat_fill_super() after error detection

[Julia Lawall]

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 linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/