[syslinux] Patch sensible callback framework

[H. Peter Anvin]

On 05/03/2010 09:20 AM, Ayvaz, James wrote:
> Resubmit with changes from Sebastian Herbszt
> +
> +callback_record* register_callback(callback_record **type, callback_t callback) {
> +    callback_record *curr, *new;
> +    if (type == NULL)
> +        return NULL;
> +    if (*type == NULL) {
> +        /* type is the head of the list */
> +        new = malloc(sizeof(callback_record));
> +        if (!new) {
> +            return NULL;
> +        }
> +        memset(new, 0, sizeof(callback_record));
> +        new->function = callback;
> +        new->prev = NULL;
> +        new->next = NULL;
> +        *type = new;
> +        return *type;
> +    }
> +    else {
> +        /* check to see if we're already registered */
> +        curr = *type;
> +        while (curr->next) {
> +            if (curr->function == callback) {
> +                return curr;
> +            }
> +            curr = curr->next;
> +        }
> +        /* add new callback entry to list */
> +        new = malloc(sizeof(callback_record));
> +        if (!new) {
> +            return NULL;
> +        }
> +
> +        memset(new, 0, sizeof(callback_record));
> +        new->function = callback;
> +        new->prev = curr;
> +        new->next = NULL;
> +        curr->next = new;
> +        return new;
> +    }
> +}


This is *way* more complex than it need to be.

First of all, I don't see much of a point to have the registration
system prevent multiple registrations, or to enforce a specific order.
Second, there is no reason to have two branches.  Third, there is no
point in using memset() since we will fill in every record in the
structure.  If there *was*, you should use calloc() or zalloc(), not
malloc+memset.

Also, checking for (type == NULL) isn't really meaningful... it's not a
very likely error and the error handling in the caller is likely to be
nonexistent.

If you *do* prevent multiple registrations, you probably would need a
registration counter which is decremented at unregister time so the
sequence register-register-unregister-unregister doesn't crash the
system, but really the better thing to do is probably to just allow the
double registration.

I still want to be able to pass a user argument to the callback, too.

It would be nice if you could submit the callback framework as a
separate patch from the actual usages of the framework.

	-hpa