Re: [PATCH 1/3] drivers/staging/cx25821: Use kstrdup

[Julia Lawall <julia@diku.dk>]

On Mon, 18 Oct 2010, Joe Perches wrote:

> On Mon, 2010-10-18 at 19:44 +0200, Julia Lawall wrote:
> > On Mon, 18 Oct 2010, Joe Perches wrote:
> > 
> > > Perhaps it's more readable code to recheck the
> > > field name flag and introduce a temporary
> > > char *fname so the slightly unusual reuse of
> > > 	field = kstrdup(field, GFP)
> > > becomes
> > > 	field = kstrdup(fname, GFP)
> > 
> > Before I had a local variable filename.  I preferred that because I felt 
> > uneasy about putting both statically and dynamically allocated memory in 
> > the same field.  But it does mean adding a new local variable.
> 
> I think readability is better using a temporary.
> 
> > I'm not sure to understand "recheck the field name flag", though.
> 
> Sorry, poor wording.  I meant using this style:
> 
> 	if (field && *field)
> 		foo
> 	else if (field)
> 		bar
> 	else
> 		baz
> 
> instead of:
> 
> 	if (field) {
> 		if (*field)
> 			foo
> 		else
> 			bar
> 	} else
> 		baz

OK :)  I'll do that.

julia