Re: [Openocd-development] [PATCH] Fix for segmentation fault from freed memory access in jtag_unregister_event_callback()

[Paul Richards]

I'm attaching the patch proposed in the last mail as I'll be offline 
shortly.

Regards,
Paul

On 2010/12/08 18:02, Paul Richards wrote:
On 2010/12/08 16:27, Peter Stuge wrote:

The priv comparison logic is reversed.

Sorry, can't believe that snuck in, thanks.

The code is horrible. (Certainly not your fault!) Sorry if that's
offensive to the original author. The variable names do not help.
Will this work:

int jtag_unregister_event_callback(callback,priv) {
   struct jtag_event_callback *p, *prev = NULL, *next;

   if (!callback)
     return ERROR_INVALID_ARGUMENTS;

   for (p = jtag_event_callbacks; p; p = next) {
     next = p->next;

     if (p->priv != priv || p->callback != callback) {
       prev = p;
       continue;
     }

     if (prev)
       prev->next = next;
     else
       jtag_events_callbacks = next;

     free(p);
   }
}


How about the following, it cleans up the naming, but does keep the 
double pointer usage which I think leaves it more compact, but 
debatably harder to read?

int jtag_unregister_event_callback(jtag_event_handler_t callback, void 
*priv)
{
    struct jtag_event_callback **p = &jtag_event_callbacks, *temp;

    if (callback == NULL)
    {
        return ERROR_INVALID_ARGUMENTS;
    }

    while (*p)
    {
        if (((*p)->priv != priv) || ((*p)->callback != callback))
        {
            p = &(*p)->next;
            continue;
        }

        temp = *p;
        *p = (*p)->next;
        free(temp);
    }

    return ERROR_OK;
}

I'm happy to remove the the double pointer if you prefer, I personally 
prefer the elegance in the compactness, but I'm happy to go along with 
what the pereferences are here.

Also I note that you use the K&R bracing, though most of the other 
code in this file doesn't.  Is there a preference?

Thanks,

Paul


From 987ecc00efaf0ee4dd2fae7b68bf4cf5a4272902 Mon Sep 17 00:00:00 2001
From: Paul Richards <paulr...@gmail.com>
Date: Wed, 8 Dec 2010 15:48:55 +0900
Subject: [PATCH] Fix for segmentation fault from freed memory access in 
jtag_unregister_event_callback()

---
 src/jtag/core.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/src/jtag/core.c b/src/jtag/core.c
index c1b64bb..f3408e1 100644
--- a/src/jtag/core.c
+++ b/src/jtag/core.c
@@ -296,28 +296,24 @@ int jtag_register_event_callback(jtag_event_handler_t 
callback, void *priv)
 
 int jtag_unregister_event_callback(jtag_event_handler_t callback, void *priv)
 {
-       struct jtag_event_callback **callbacks_p;
-       struct jtag_event_callback **next;
+       struct jtag_event_callback **p = &jtag_event_callbacks, *temp;
 
        if (callback == NULL)
        {
                return ERROR_INVALID_ARGUMENTS;
        }
 
-       for (callbacks_p = &jtag_event_callbacks;
-                       *callbacks_p != NULL;
-                       callbacks_p = next)
+       while (*p)
        {
-               next = &((*callbacks_p)->next);
-
-               if ((*callbacks_p)->priv != priv)
-                       continue;
-
-               if ((*callbacks_p)->callback == callback)
+               if (((*p)->priv != priv) || ((*p)->callback != callback))
                {
-                       free(*callbacks_p);
-                       *callbacks_p = *next;
+                       p = &(*p)->next;
+                       continue;
                }
+
+               temp = *p;
+               *p = (*p)->next;
+               free(temp);
        }
 
        return ERROR_OK;
-- 
1.7.2.3

_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development