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

  • From: Xin Gu <eric.nevup@xxxxxxxxx>
  • To: hipl-dev@xxxxxxxxxxxxx
  • Date: Tue, 17 Apr 2012 19:14:51 +0300

Hi,

On 19/03/12 16:59, Diego Biurrun wrote:
  review needs-fixing


--- lib/core/hostid.c   2012-03-01 14:26:43 +0000
+++ lib/core/hostid.c   2012-03-15 08:58:36 +0000
@@ -678,6 +679,7 @@
      int                      dsa_pub_key_rr_len = 0, rsa_pub_key_rr_len = 0;
      hip_hdr                  numeric_action     = 0;
+    char                    *hi_file_dup        = NULL;
      char                     hostname[HIP_HOST_ID_HOSTNAME_LEN_MAX];
@@ -743,11 +745,16 @@
                  goto out_err;
              }
          } else if (!use_default) {
+            hi_file_dup = strdup(hi_file);
+            if ((err = check_and_create_dir(dirname(hi_file_dup), 
HIP_DIR_MODE))) {
+                HIP_ERROR("Could not create direcory for path: %s\n", hi_file);
+                goto out_err;
You strup - why?

the function dirname() might modify the parameter, so I copy it.


unrelated: I'm slightly suspicious of HIP_DIR_MODE, which is 0755.
Is it really safe to make this world-readable?


It seems to be problematic, what should be a safe DIR_MODE?


@@ -523,6 +551,208 @@
+static int libhip_init_handle_functions(void)
+{
+    //hip_register_handle_function(HIP_I2, 
HIP_STATE_UNASSOCIATED,&hip_setup_ipsec_sa, 30500);
+    //hip_register_handle_function(HIP_I2, 
HIP_STATE_I1_SENT,&hip_setup_ipsec_sa, 30500);
+    //hip_register_handle_function(HIP_I2, 
HIP_STATE_I2_SENT,&hip_setup_ipsec_sa, 30500);
+    //hip_register_handle_function(HIP_I2, 
HIP_STATE_R2_SENT,&hip_setup_ipsec_sa, 30500);
+    //hip_register_handle_function(HIP_I2, 
HIP_STATE_ESTABLISHED,&hip_setup_ipsec_sa, 30500);
+    //hip_register_handle_function(HIP_I2, 
HIP_STATE_CLOSING,&hip_setup_ipsec_sa, 30500);
+    //hip_register_handle_function(HIP_I2, 
HIP_STATE_CLOSED,&hip_setup_ipsec_sa, 30500);
+    //hip_register_handle_function(HIP_I2, HIP_STATE_NONE,&hip_setup_ipsec_sa, 
30500);
+    //hip_register_handle_function(HIP_R2, 
HIP_STATE_I2_SENT,&hip_setup_ipsec_sa, 30500);
Why are all these commented-out?

Because kernel-based IPSec functions cannot be used in libhipl. Now I delete those lines instead of commenting out.



All other issues are fixed.
Although hipnetcat and its test cases are not included in the merge proposal this time, I fixed those issues you mentioned. You will see them when I merge the hipnetcat.

Thanks for the review.

Xin

Other related posts: