Hi, On 26/02/12 10:25, Stefan Götz wrote:
+ * @param ptr pointer by which to identify the node + * @param free_element a function pointer to a function for freeing the memory + * allocated for an element at a node or NULL if the element + * itself is not to be freed. + */ +void hip_ll_del_by_ptr(struct hip_ll *linkedlist, void *ptr, + free_elem_fn free_element)const correctness: struct hip_ll *const linkedlist, const void *const ptr
I was wondering if we can just use "const void *ptr"? I haven't figured out why we need the second const.
If I understand correctly, you also suggest to change "struct hip_packet_context *ctx" to "struct hip_packet_context *const ctx" for the following function. Then I have the same question related to const after asterisk.
int hip_read_control_msg_tcp(int sockfd, struct hip_packet_context *ctx)
Why not go with Christof's version? It looked much simpler to me.
Current version is already based on Christof's advice. It seems that somehow you receive an old version. I don't know why...
--- hipd/hadb.c 2012-02-17 10:45:47 +0000 +++ lib/hipdaemon/hadb.c 2012-02-20 08:33:22 +0000 @@ -616,7 +616,12 @@ if (hip_select_source_address(&peer_map.our_addr,&peer_map.peer_addr)) { HIP_ERROR("Cannot find source address\n"); - return -1; + if (hipl_is_libhip_mode()) { + memset(&peer_map.our_addr, 0, sizeof(peer_map.our_addr));How about 'peer_map.our_addr = INVALID_ADDR' with a suitable definition (and probably a better name for) INVALID_ADDR? It would reveal the intention of this code more clearly+ HIP_DEBUG("Using ANY for source address\n"); + } else { + return -1; + }
Hmm, what about add a comment? I add a comment and also rearrange this part a bit, hope it is more clear.
I couldn't finish reviewing, maybe later. In any case, const correctness for all further functions, please :)
I revised other code related to const correctness. Thanks for the review :)https://code.launchpad.net/~hipl-core/hipl/libhip The diff in the "Branch merges" section of this page is up-to-date.
Xin