Re: 2.6.38 nfsd bugfixes

[Linus Torvalds]

On Wed, Feb 16, 2011 at 8:25 PM, J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote:
> -               if ((host_err = nfsd_map_name_to_uid(argp->rqstp, buf, dummy32, &iattr->ia_uid)))
> -                       goto out_nfserr;
> +               if ((status = nfsd_map_name_to_uid(argp->rqstp, buf, dummy32, &iattr->ia_uid)))
> +                       return status;

Btw, can we please just agree to not doing those idiotic double parenthesis?

There is a really trivial solution to the gcc warning - write your
code like a sane person, instead of some ex-LISP hacker that has
withdrawal symptoms. IOW, the above should be written as

  status = nfsd_map_name_to_uid(argp->rqstp, buf, dummy32, &iattr->ia_uid);
  if (status)
    return status;

which is a hell of a lot more readable, no?

There is never any real excuse to put an assignment inside a regular
if-statement.

Inside a while/for loop? Sure. There are real syntactic reasons for
doing things like

   while ((c = getchar()) != EOF) {
   }

that actually make the code better and denser and avoid extra control
flow crap or duplicate code.

Inside a macro expansion? Again, there may be good reasons to try to
make it a single statement.

But a simple if-statement? There just isn't any reason for it, since
the obvious thing is to just write it as two separate statements: the
assignment, and the if-statement. So why do it and make the code
uglier and harder to parse?

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