linux-kernel - Re: 2.6.38 nfsd bugfixes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTikeK77v7y346-OX9dvufJSjbJ7qQSpJc4C3wLrZ@mail.gmail.com>
Date:	Wed, 16 Feb 2011 20:32:06 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	"J. Bruce Fields" <bfields@...ldses.org>
Cc:	linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: 2.6.38 nfsd bugfixes

On Wed, Feb 16, 2011 at 8:25 PM, J. Bruce Fields <bfields@...ldses.org> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/