[hipl-dev] Re: [Merge] lp:~stefan.goetz/hipl/hidb-db into lp:hipl

  • From: David Martin <david.martin.mailbox@xxxxxxxxxxxxxx>
  • To: mp+61832@xxxxxxxxxxxxxxxxxx
  • Date: Fri, 03 Jun 2011 14:02:29 -0000

Review: Needs Information
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.
-- 
https://code.launchpad.net/~stefan.goetz/hipl/hidb-db/+merge/61832
Your team HIPL core team is subscribed to branch lp:hipl.

Other related posts: