[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150405174531.GA8664@openwall.com>
Date: Sun, 5 Apr 2015 20:45:31 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: Coding Style
On Sun, Apr 05, 2015 at 08:28:06PM +0300, Alexander Cherepanov wrote:
> I don't think such issues could be discussed in isolation. One of the
> peculiarities of your style is the absence of indent in the following
> code (from http://www.openwall.com/lists/john-dev/2011/07/15/2 ):
>
> for (i = 0; i < 10; i++)
> for (j = 0; j < 10; j++)
> for (k = 0; k < 10; k++) {
> ... body ...
> ... body ...
> }
>
> This compensates for wide tabs to some degree. But I find this approach
> very strange. Surely, the example above looks nice. But your next example:
>
> for (i = 0; i < 10; i++)
> if (ok(i))
> while (j))
> ... body ...
> ... body ...
> }
>
> doesn't look better without indentation.
To me, it looks fine without indentation (but you lost the opening curly
brace there). I simply treat the for/if/while lines of it as one
complicated loop/condition for the following block.
> And combined with the fact that there are fragments like this:
>
> for (pos = &ciphertext[4]; atoi16[ARCH_INDEX(*pos)] != 0x7F; pos++);
> if (*pos || pos - ciphertext != 20) return 0;
These are actually no good. Today, I'd format them as:
for (pos = &ciphertext[4]; atoi16[ARCH_INDEX(*pos)] != 0x7F; pos++)
continue;
if (*pos || pos - ciphertext != 20)
return 0;
or better yet, rewrite the "for" loop as "while":
pos = ciphertext + 4;
while (atoi16[ARCH_INDEX(*pos)] != 0x7F)
pos++;
BTW, we need to replace ARCH_INDEX() with an arch-neutral macro, perhaps
in common.h.
> it greatly undermines the usefulness of indentation -- you cannot be
> sure anymore where a block ended without fully parsing all the code.
With the above corrections, I think you can. So I think corrections
like these is what we should make.
> I think the principle that you can find the end of the block just by
> mentally drawing a vertical line until you meet something with less or
> equal indentation
This is a required property, yes.
> overweights the usefulness of alignment like the first example above.
What "first example above"? The one with "for ... ;" all on one line?
Yes, we should get rid of those.
Alexander
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
