Code review comment for lp:~fahad-aizaz/hipl/hip-cert-conf

Diego Biurrun (diego-biurrun) wrote :

 review needs-info

On Wed, Jan 25, 2012 at 03:14:31PM +0000, Fahad Aizaz wrote:
>
> Note:
> While doing so, I observed that the configuration file though
> referenced in the code for reading, but I didn't find any trigger to
> actually read the contents of the file. That is, the function that
> reads the configuration file section by section, is not being called
> though its definition exist.

What function are you talking about? Please provide details, do not
let us guess.

The patch itself looks OK, but this is an issue that we will of course
have to sort out.

> --- hipd/init.c 2011-12-13 13:50:53 +0000
> +++ hipd/init.c 2012-01-25 15:13:34 +0000
> @@ -924,7 +819,7 @@
> int hipd_init(const uint64_t flags)
> {
> - int err = 0, certerr = 0, i, j;
> + int err = 0, i, j;
> int killold = (flags & HIPD_START_KILL_OLD) > 0;
> @@ -1065,12 +960,6 @@
>
> - certerr = 0;
> - certerr = init_certs();
> - if (certerr < 0) {
> - HIP_DEBUG("Initializing cert configuration file returned error\n");
> - }

So certerr used to get initialized a full *three* times? This is, ummm,
suboptimal even by HIPL standards ...

Diego

review: Needs Information

« Back to merge proposal