Re: [patch 1/8] isdn_bsdcomp.c - vfree() checking cleanups

[Karsten Keil <kkeil@suse.de>]

On Mon, Mar 07, 2005 at 01:21:33AM +0100, Domen Puncer wrote:
> On 07/03/05 00:07 +0000, Ralph Corderoy wrote:
> > 
> > Hi Domen,
> > 
> > > -		if (db->dict) {
> > > -			vfree (db->dict);
> > > -			db->dict = NULL;
> > > -		}
> > > +		vfree (db->dict);
> > > +		db->dict = NULL;
> > 
> > Is it really worth always calling vfree() which calls __vunmap() before
> > db->dict is determined to be NULL in order to turn three lines into two?
> 
> Four lines into two :-)
> 
> > Plus the write to db->dict which might otherwise not be needed.  The old
> > code was clear, clean, and fast, no?
> 
> Shorter and more readable code is always better, right? And speed really
> doesn't seem to be an issue here.
> 

I also prefer the old code, since it make clear, that you must be careful
here, since the function can be called with already freed db->dict, and for
me this version is not better readable as the old one.

-- 
Karsten Keil
SuSE Labs
ISDN development