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

Revision history for this message
Miika Komu (miika-iki) wrote :

Hi,

On 08/11/11 18:16, Diego Biurrun wrote:
> 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.

done, changed this to "1" and adjusted trunk code to match this new policy.

« Back to merge proposal