Code review comment for lp:~hipl-core/hipl/ecdsa-redhat

On Thu, Nov 03, 2011 at 10:07:31PM +0000, Diego Biurrun wrote:
> On Thu, Nov 03, 2011 at 11:37:40PM +0200, Miika Komu wrote:
> > On 11/03/2011 07:38 PM, Diego Biurrun wrote:
> > >On Thu, Nov 03, 2011 at 03:01:41PM +0000, Miika Komu wrote:
> > >>--- firewall/rule_management.c 2011-08-15 14:11:56 +0000
> > >>+++ firewall/rule_management.c 2011-11-03 15:00:34 +0000
> > >>@@ -444,6 +443,7 @@
> > >>
> > >>+#ifdef HAVE_EC_CRYPTO
> > >> /**
> > >> * Load an ECDSA public key from a file and convert it into a hip_host_id.
> > >> *
> > >>@@ -479,6 +479,8 @@
> > >> return err;
> > >> }
> > >>
> > >>+#endif /* HAVE_EC_CRYPTO */
> > >>+
> > >> /**
> > >> * load a public key from a file and convert it to a hip_host_id structure
> > >> *
> > >
> > >Please drop the empty line befor the #ifdef.
> > >
> > >same elsewhere
> >
> > I thought this was the policy, observe bzr commit:
> >
> > The following differences were found between the code to commit and
> > the rules in '.uncrustify-0.57.cfg':
> >
> > ---
> > /home/mkomu/projects/hipl-bzr/ecdsa-redhat/firewall/rule_management.c
> > 2011-11-03 23:08:00.909848709 +0200
> > +++ - 2011-11-03 23:29:16.724968313 +0200
> > @@ -479,6 +479,7 @@
> > free(ecdsa_key_rr);
> > return err;
> > }
> > +
> > #endif /* HAVE_EC_CRYPTO */
> >
> > /**
> >
> > The above changes are also available in the file /tmp/stylecheck-diff-L4x2dJ
> >
> > What to do about this... any help?
>
> It seems our uncrustify configuration is in need of fixing.
> I'll look into it next week - harass me if I don't.

It's the following setting:

  # The number of newlines after '}' of a multi-line function body
  nl_after_func_body = 2 # number

This will force an empty line between functions. It does not take #endifs
into account, however. I don't much care how we handle this. We can
either live with the newline before the #endif or drop the setting.
Make your choice.

Diego