hipl-core team mailing list archive

[Christof Mroz <christof.mroz@xxxxxxxxxxxxxx>]

On Thu, 26 Aug 2010 17:33:34 +0300
Miika Komu <mkomu@xxxxxxxxx> wrote:

> maybe the context should be reinitialized after processing a packet
> (successfully or not)?

I agree. See my response to Réne's mail for reasons on why I chose the
location in the patch below.

> It seems to be initialized at least here but maybe it's not enough:
> 
> mkomu@gaijin:~/projects/hipl-bzr/hipl$ grep -rn ctx hipd |grep memset
> ..
> hipd/hipd.c:296:    memset(&ctx, 0, sizeof(ctx));
> ..

That's the very first initialization (right after allocation), i.e.
it's executed exactly once, so it certainly isn't enough.

I propose the following:

=== modified file 'hipd/hip_socket.c'
--- hipd/hip_socket.c	2010-08-29 16:24:14 +0000
+++ hipd/hip_socket.c	2010-08-30 08:39:37 +0000
@@ -260,8 +260,14 @@
             socketfd = ((struct socketfd*) iter->ptr)->fd;
 
             if (FD_ISSET(socketfd, read_fdset)) {
-                ctx->error = 0;
                 ((struct socketfd*) iter->ptr)->func_ptr(ctx);
+                HIP_DEBUG("result: %d\n", ctx->error);
+
+                /* clear pointers only; if any memory was allocated in handler,
+                 * it's supposed to have been freed there already. */
+                ctx->output_msg = NULL;
+                ctx->hadb_entry = NULL;
+                ctx->error      = 0;
             }
         }
     } else {

=== modified file 'hipd/hipd.c'
--- hipd/hipd.c	2010-08-25 09:06:43 +0000
+++ hipd/hipd.c	2010-08-27 17:36:52 +0000
@@ -434,21 +434,17 @@
     /* free allocated resources */
     hip_exit();
 
-    if (ctx.input_msg) {
-        free(ctx.input_msg);
-    }
-
-    if (ctx.src_addr) {
-        free(ctx.src_addr);
-    }
-
-    if (ctx.dst_addr) {
-        free(ctx.dst_addr);
-    }
-
-    if (ctx.msg_ports) {
-        free(ctx.msg_ports);
-    }
+    /* if any of these fail, something went wrong along the way */
+    HIP_ASSERT(!ctx.output_msg);
+    HIP_ASSERT(!ctx.hadb_entry);
+    HIP_ASSERT(ctx.input_msg);
+    HIP_ASSERT(ctx.src_addr);
+    HIP_ASSERT(ctx.dst_addr);
+    HIP_ASSERT(ctx.msg_ports);
+    free(ctx.input_msg);
+    free(ctx.src_addr);
+    free(ctx.dst_addr);
+    free(ctx.msg_ports);
 
     HIP_INFO("hipd pid=%d exiting, retval=%d\n", getpid(), err);