[hipl-dev] Re: [Merge] lp:~hipl-core/hipl/libhip into lp:hipl

  • From: Xin Gu <eric.nevup@xxxxxxxxx>
  • To: hipl-dev@xxxxxxxxxxxxx
  • Date: Sun, 26 Feb 2012 21:11:44 +0200

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

Other related posts: