lkml.org 
[lkml]   [2005]   [Mar]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] remove redundant NULL checks before kfree() in drivers/video/
On Mon, 21 Mar 2005, Antonino A. Daplas wrote:

> On Sunday 20 March 2005 06:59, Jesper Juhl wrote:
> > Checking a pointer for NULL before calling kfree() on it is redundant,
> > kfree() deals with NULL pointers just fine.
> > This patch removes such checks from files in drivers/video/
> >
> > Since this is a fairly trivial change (and the same change made
> > everywhere) I've just made a single patch for all the files and CC all
> > authors/maintainers of those files I could find for comments. If spliting
> > this into one patch pr file is prefered, then I can easily do that as
> > well.
> >
>
> [snip]
>
> > --- linux-2.6.11-mm4-orig/drivers/video/console/bitblit.c 2005-03-16
> > 15:45:26.000000000 +0100 +++
> > linux-2.6.11-mm4/drivers/video/console/bitblit.c 2005-03-19
> > 22:27:39.000000000 +0100 @@ -199,8 +199,7 @@ static void bit_putcs(struct
> > vc_data *vc
> > count -= cnt;
> > }
> >
> > - if (buf)
> > - kfree(buf);
> > + kfree(buf);
> > }
> >
>
> This is performance critical, so I would like the check to remain. A comment
> may be added in this section.
>
Ok, I believe Andrew already merged the patch into -mm, if you really want
that check back then I'll send him a patch to put it back and add a
comment once he puts out the next -mm.
But, at the risk of exposing my ignorance, I have to ask if it wouldn't
actually perform better /without/ the if(buf) bit? The reason I say that
is that the generated code shrinks quite a bit when it's removed, and also
kfree() itself does the same NULL check as the very first thing, so it
comes down to the bennefit of shorter generated code, one less branch,
against the overhead of a function call - and how often will 'buf' be
NULL? if buff is != NULL the majority of the time, then it should be a
gain to remove the if().


> > static void bit_clear_margins(struct vc_data *vc, struct fb_info *info,
> > @@ -273,8 +272,7 @@ static void bit_cursor(struct vc_data *v
> > dst = kmalloc(w * vc->vc_font.height, GFP_ATOMIC);
> > if (!dst)
> > return;
> > - if (ops->cursor_data)
> > - kfree(ops->cursor_data);
> > + kfree(ops->cursor_data);
> > ops->cursor_data = dst;
> > update_attr(dst, src, attribute, vc);
> > src = dst;
> > @@ -321,8 +319,7 @@ static void bit_cursor(struct vc_data *v
> > if (!mask)
> > return;
> >
> > - if (ops->cursor_state.mask)
> > - kfree(ops->cursor_state.mask);
> > + kfree(ops->cursor_state.mask);
> > ops->cursor_state.mask = mask;
>
> Although these are also performance critical, I will agree that the checks
> can go. Very rarely will ops->cursor_state.mask and ops->cursor_data be
> NULL.
>
> As for the rest, they are acceptable, as long as the maintainers agree.
>
Ok, thank you for commenting.


--
Jesper Juhl

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:11    [W:0.062 / U:0.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site