aklog: Don't reference freed node whilst deleting
authorSimon Wilkinson <sxw@your-file-system.com>
Fri, 15 Feb 2013 16:23:16 +0000 (16:23 +0000)
committerDerrick Brashear <shadow@your-file-system.com>
Thu, 21 Feb 2013 13:16:32 +0000 (05:16 -0800)
Because deletion is implemented using a for loop, the step of the
loop that moves us to the next node references freed memory when
we've deleted an element. Fix this by just shortcircuiting the
return from the function so we immediately exit.

Change-Id: Ia820b20ce5937ac86d849cb746b3bc21f46550fa
Reviewed-on: http://gerrit.openafs.org/9161
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>

src/aklog/linked_list.c

index 0649482..289d645 100644 (file)
@@ -101,38 +101,31 @@ int ll_delete_node(linked_list *list, ll_node *node)
    *   won't point to valid data.
    */
 {
-    int status = LL_SUCCESS;
     ll_node *cur_node = NULL;
-    int found = FALSE;
 
     if (list->nelements == 0)
-       status = LL_FAILURE;
-    else {
-       for (cur_node = list->first; (cur_node != NULL) && !found;
-            cur_node = cur_node->next) {
-           if (cur_node == node) {
+       return LL_FAILURE;
 
-               if (cur_node->prev)
-                   cur_node->prev->next = cur_node->next;
-               else
-                   list->first = cur_node->next;
+    for (cur_node = list->first; cur_node != NULL; cur_node = cur_node->next) {
+       if (cur_node == node) {
 
-               if (cur_node->next)
-                   cur_node->next->prev = cur_node->prev;
-               else
-                   list->last = cur_node->prev;
+           if (cur_node->prev)
+               cur_node->prev->next = cur_node->next;
+           else
+               list->first = cur_node->next;
 
-               free(cur_node);
-               list->nelements--;
-               found = TRUE;
-           }
+           if (cur_node->next)
+               cur_node->next->prev = cur_node->prev;
+           else
+               list->last = cur_node->prev;
+
+           free(cur_node);
+           list->nelements--;
+           return LL_SUCCESS;
        }
     }
 
-    if (!found)
-       status = LL_FAILURE;
-
-    return(status);
+    return LL_FAILURE;
 }