Code review comment for lp:~stefan.goetz-deactivatedaccount/hipl/hidb-db

Revision history for this message
David Martin (martin-lp) wrote :

Hi,
after having looked through this branch I have one question and only a single remark on your code:

In revision 5941:
> Remove the locking macros for the HIDB because they are not used.

What do you mean with "they are not used"? Because there are lots of calls to lock / unlock that
you remove and without looking deeper into it seems like it is used here. Or is it because all
access is done on our local hidb and we are neither multi-threaded nor does it get accessed from
beyond the file so no locking is necessary?

> -struct local_host_id *hip_get_hostid_entry_by_lhi_and_algo(HIP_HASHTABLE *db,
> - const struct in6_addr *hit,
> +struct local_host_id *hip_get_hostid_entry_by_lhi_and_algo(const struct in6_addr *hit,

"const struct in6_addr *const hit" would be more correct here?

> +/**
> + * A call-back function for finding a host association between a given peer HIT
> + * and any of the iteratively provided local HIs.
> + *
> + * @param lhi Points to a local_host_id object from which contains the
> + * local HIT of the HA to find.

"Points to a local_host_id object which contains"

> -int hip_hidb_match(const void *ptr1, const void *ptr2)
> +static int hip_hidb_match(const void *ptr1, const void *ptr2)

*const ptr1 and *const ptr2 would be more correct?

Other than that, as far as I'm able to judge it's looking good. Nice one! :)

PS: hipd crashes on shutdown after doing a base exchange. This has already been fixed in trunk revision 5948 and applying the change fixes it good.

review: Needs Information

« Back to merge proposal