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

[Paul Richards]

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

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