views:

52

answers:

2

My library offers a callback point where my library users can register to get information. The general form of the callback is an int followed by various parameters whose type depend on the int value. Therefore, I defined the callback type and the function to set it as follows.

typedef void (* callback_func_t)(int type, ...);
void set_callback_func(callback_func_t callback);

Within my library, I am calling this function all throughout, being the user set function, or the default one I am providing. It works.

Now, I'd like to add a level of indirection, to be able to call several registered callbacks. The trouble is that my internal callback function that still takes ellipsis parameters, also have to call the callback function with ellipsis. As a consequence, my internal function has to interpret the type, unpack the parameters from the va_list and give them as parameters to the callbacj function.

void internal_callback(int type, ...) {
    va_list args;
    va_start(args, type);
    switch (type) {
    case 0: call_callback(type, va_arg(args, int)); break;
    // ...
    }
    va_end(args);
}

But then, in the user's implementation of the callback, there will also be the same va_list usage, and interpretation of the arguments, according to the value of type. The solution is to pass directly the va_list as an argument to the callback function, making the implementation of the internal callback function obvious.

typedef void (* callback_func_t)(int type, va_list args);

My question is: is it good practice to define a callback function type that takes va_list as argument? I can define my callback function type as above, but what are the pros and cons compared to the definition at the top?

+3  A: 

I assume there is a finite and known number of types? If so, why not use enumerations and unions for some level of type safety, which would incidentally also solve your problem:

enum callback_arg_type { INT_ARG, DOUBLE_ARG, VECTOR_ARG };

union callback_arg
{
    int as_int;
    double as_double;
    struct { double x, y, z; } as_vector;
};

typedef void (*callback_func_t)(
    enum callback_arg_type type, union callback_arg arg);

Depending on the difference of argument sizes, it might be a good idea to pass a pointer instead. You could also provide some macros to provide a nicer syntax for callback invocation, but if they are only ever called from within your library, it might not be worth it:

union callback_arg arg = { .as_int = 42 };
callback_fn(INT_ARG, arg);

is pretty short on its own.

Christoph
+2  A: 

I would go with the va_list version. The user is already dealing with the scariness of dealing with va_lists so you don't save anything by hiding that from them.

Additionally using the list rather than trying to repass the arguments will cut down on stack usage. If you had to repass the arguments to their function it would make a new copy of all of those arguments, but passing the va_list type will just use the instance of those arguments already on the stack.

Finally, your code will both be simpler (if I'm understanding the problem correctly) and you will save each user from having to call va_start and va_end (which probably won't change the output code in their function much for most implementations of stdarg) but otherwise they all have to type those calls and (depending on how stdargs are implemented on the platform) they will need to make sure they actually do call va_end before returning (easy to miss if returning early in case of an error).

nategoose