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

[Jörn Engel]

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? ;)

Jörn

-- 
The competent programmer is fully aware of the strictly limited size of
his own skull; therefore he approaches the programming task in full
humility, and among other things he avoids clever tricks like the plague. 
-- Edsger W. Dijkstra
-
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/