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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
