Re: Indentation and coding style of imported files (was: [nmap-svn] r29240)

[David Fifield <david () bamsoftware com>]

On Wed, Jul 18, 2012 at 12:58:31PM +0200, Henri Doreau wrote:
> 2012/7/17  <commit-mailer () insecure org>:
> > Author: perdo
> > Date: Tue Jul 17 11:07:28 2012
> > New Revision: 29240
> >
> > Log:
> > Add parts of LuaFileSystem to NSE (support for mkdir, rmdir, link).
> >
> > Added:
> >    nmap/nselib/lfs.luadoc
> > Modified:
> >    nmap/nse_fs.cc
> >    nmap/nse_fs.h
> >    nmap/nse_main.cc
> >    nmap/nse_main.lua
> > [...]
>
> Hello,
>
> I noticed that indentation and bracket style of nse_fs.cc lacks
> consistency. It seems that it was originally using tabs, and some
> modifications were made with two-spaces indented lines. I think we'd
> all agree that indentation and coding style should be unified, but
> given that this file was imported from another project (if I
> understood correctly) I don't know what's the best way to do it.
>
> Is there already a policy? Should we establish one? A couple other nse
> files would benefit this.
>
> At the moment, most C and C++ files use something like:
> $ indent -nut -i2 -kr -br -brs -brf -l0 -bad -npcs -nprs -ncs <file>
>
> Obviously, enforcing this project wise code convention would be
> interesting (my favorite option actually) but OTOH not changing the
> indentation of imported files also makes sense as it eases tracking
> and porting upstream changes.

I agree we should make the indentation style of imported code match.

I would love to automatically convert all our source files a common
style. (Or common-ish; Ncat C is different from Nmap C++, and I think
that's fine.) What has prevented me is that every automatic indenter
I've tried has required a large amount of manual cleanup. Here are some
things I have tried that are in the revision log:

r15356
        indent -kr -i2 -nut -brf tcpip.cc
r28337, r28772
        astyle -a -p -H -s2 -m2 tcpip.cc scan_engine.cc
r28984 (Ncat C style, not C++)
        indent -kr -i4 -nut -l0 -ss -T size_t *.c

The things that usually need cleanup are:
* Knowledge of type names; otherwise indenters make pointer syntax look
  like multiplication: "size_t * len" instead of "size_t *len". This can
  be solved in indent with an exhaustive list of type names we use
  ("-T size_t"). astyle might do it automatically, I'm not sure.
* Breaking long lines. Personally I prefer to have a line of 85
  characters rather than break it into two lines. Sometimes it's better
  to break the line though; I didn't find a tool that made the right
  call most of the time.

I wouldn't be opposed to a wholesale conversion of all our source files,
even if there are some warts remaining. It should come with a script to
run to prove that the modified source files produce the same assembly
code.

David Fifield
_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/