search

openssl / web

www.openssl.org
52 stars 76 forks source link

The Coding Style guidelines and util/indent.pro need an update #138

Open DDvO opened 5 years ago

DDvO commented 5 years ago

During submission of the first chunks of our CMP contribution I learned that there are some (implicit) coding-style rules that are not (yet) part of the official coding guideline at https://www.openssl.org/policies/codingstyle.html. In particular:

Are there further such implicit rules to be followed? Please update that Coding Style document accordingly.

Meanwhile we found that there is automated tool support for (re-)indenting source files: the indent configuration file util\indent.pro, which is used by the util/openssl-format-source script.

Apparently this tool has not been used on many of the OpenSSL source files. It this tool recommended to use, at least for new source files?

If so, also util\indent.pro should be updated to reflect all coding style rules, also those that are so far implicit. In particular, the above rule on && and || is not reflected there. Would it be possible to state it as an indent rule, and if so, how to do it?

levitte commented 5 years ago
  • in multi-line Boolean expressions, any && and || operators should not be given at the end of lines but at the beginning of the following line, with and extra indentation of 4 spaces.

This has actually been left open ended on purpose. However, it seems like we have at least all zeroed in on this:

The extra indentation is something we don't all agree on, so it has stayed open ended, i.e. we allow it but don't make it mandatory. If you have looked at my C code, you'll notice that I don't do that extra indentation, and it's for a very simple reason: there's no way that I know of to make indentation helpers to add that extra indentation without deep hackery, and I for one have no desire whatsoever to battle cc-mode or indent over this.

levitte commented 5 years ago

All that being said, I agree that our coding style guide needs an update.

DDvO commented 5 years ago

Thanks @levitte for your comments. Glad that the extra 4 spaces before && and || are not mandatory ;-)

Trying the tool mentioned above on the (already approved and merged) header file include/openssl/crmf.h it turns out that most declarations receive an extra indent of 4 spaces and comments get moved to the right. I do not think this is on purpose - how to avoid this?