On Mon, 7 Mar 2005, J=F6rn Engel wrote:

> On Mon, 7 March 2005 03:28:46 -0700, Andreas Dilger wrote:
> >=20
> > Ironically, the whitespace patch gets the small things right, but misse=

s
> > 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.
> >=20
> > 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.
> >=20
> > 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.

>=20
> 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.
>=20
> Unless my glass ball needs cleaning again. Jesper?
>=20

That is the plan. Small steps.=20

--=20
Jesper