Re: [PATCH] whitespace cleanups for fs/cifs/file.c

[Jesper Juhl]

On Mon, 7 Mar 2005, Jörn Engel wrote:

> On Mon, 7 March 2005 03:28:46 -0700, Andreas Dilger wrote:
> > 
> > Ironically, the whitespace patch gets the small things right, but misses
> > on the big readability issues, such as cifs_open() being 220 lines long
> > and having a _really_ hard time staying inside 80 columns because of so
> > many levels of nested conditionals.
> > 
> > Judicious use of gotos and some helper functions would help a lot
> > here (e.g.  after CIFSSMBOpen() "if (rc) { ... goto out; }" and
> > "if (!file->private_data) goto out;", would avoid indenting the rest
> > of the function 16 columns.  Adding a couple helper functions like
> > "cifs_convert_flags()" to return desiredAccess and disposition, and
> > "cifs_init_private_data()" to allocate ->private_data and initialize
> > the masses of fields would be good.
> > 
> > Is it possible that pCifsInode can ever be NULL???  Similarly, "if (buf)"
> > on line 196 is needless, as it has already been checked on line 153
> > (and we abort in that case).  Also, kfree() can handle NULL pointers.
> 
> Jesper knows those problems already (at least some of them).  Right
> now, his biggest problem appears to be patch submission.  As soon as
> Steve accepts his patches and the backlog is shrinking, he might get
> to those issues, one at a time.
> 
> Unless my glass ball needs cleaning again.  Jesper? ;)
> 
That is the plan. Small steps. 

-- 
Jesper

-
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/