Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150405120413.GA5108@openwall.com>
Date: Sun, 5 Apr 2015 15:04:14 +0300
From: Solar Designer <solar@...nwall.com>
To: john-dev@...ts.openwall.com
Subject: Re: Coding Style

On Sun, Apr 05, 2015 at 12:43:23PM +0200, magnum wrote:
> On 2015-04-05 12:02, Solar Designer wrote:
> > As discussed before, this is about the closest match to what we need:
> > 
> > 	indent -kr -i8 -nlp -nbbo -ncs -l79 -lc79
> > 
> > although I dislike a few things about it - e.g., how it indents goto
> > labels.  Oh, and IIRC our fmt_main struct initializers are mangled by
> > this really bad.
> > 
> > Maybe we should patch indent(1) for our use?
> 
> I think GNU indent has more options that are applicabe to us, than the
> most basic variants. I *think* I could get around the struct issue but I
> don't have any notes left from that. Someone should investigate what can
> be used (and we should list it as an alternative for GNU indent only)

I think it'd be helpful if you or Kai or someone else investigates this
and posts in here.  The ideal indent program and its options would
produce results close to the current formatting of the core tree.

> > Another drawback of this mass-reformat is that the core tree of JtR
> > intentionally uses custom formatting in a few places.  I guess I'd have
> > to give up on this, just to avoid unnecessary diffs with jumbo.
> 
> Speaking of core. Jumbo should strive to not change core, only add to it
> (when possible, of course). For this reason my patches to core files
> often has non-standard formatting too, eg.
> 
>         crk_key_index = 0;
>         crk_last_salt = NULL;
> +       if (options.flags & FLG_MASK_STACKED)
> +               mask_fix_state();
> +       else
>         crk_fix_state();
> 
> 
> Here, the crk_fix_state was not indented - and this was intentional. The
> more we do it like that, the less merge conflicts I get when merging core.

Yes, but I dislike improper indentation in jumbo like that.  I think we
should prioritize proper indentation over having fewer merge conflicts.

> BTW I am tempted to officially have Jumbo deviate from core's tab width
> recommentdation, using 4 instead of 8. With the "indent with TABs, align
> with spaces" requirement (which I'm prepared to kill for) this only
> affects how long a line gets[*]. It seems to me absolutely no-one use 8
> (except I use to do it when working with John - but I'm about to give
> that up) so many lines are committed that did not exceed column 80 with
> the author's TAB setting but do when 8 is used.

I use 8 char tabs, and I like them.  In early 1990s, I used to prefer
indentation with a few spaces (2 or 3), but later I realized that if I
need so many indentation levels that 8 char tabs start being a problem
(push lines beyond 80 chars), this usually means that the code needs to
be refactored (usually split into more functions).

8 char tabs are used in Linux kernel, and in many *BSD's KNL
http://en.wikipedia.org/wiki/Kernel_Normal_Form

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.