Declaration, allocation and assignment of an array of pointers to function pointers

Question

Hello Stack Overflow! This is my first post, so please be gentle.

I've been playing around with C from time to time in the past. Now I've gotten to the point where I've started a real project (a 2D graphics engine using SDL, but that's irrelevant for the question), to be able to say that I have some real C experience. Yesterday, while working on the event system, I ran into a problem which I couldn't solve.

There's this typedef,

//the void parameter is really an SDL_Event*.  
//but that  is irrelevant for this question.  
typedef void (*event_callback)(void);  

which specifies the signature of a function to be called on engine events.

I want to be able to support multiple event_callbacks, so an array of these callbacks would be an idea, but do not want to limit the amount of callbacks, so I need some sort of dynamic allocation. This is where the problem arose. My first attempt went like this:

//initial size of callback vector  
static const int initial_vecsize = 32;  
//our event callback vector  
static event_callback* vec = 0;  
//size  
static unsigned int vecsize = 0;  

void register_event_callback(event_callback func) {  
    if (!vec)  
        __engine_allocate_vec(vec);  
    vec[vecsize++] = func; //error here!  
}  

static void __engine_allocate_vec(engine_callback* vec) {  
    vec = (engine_callback*) malloc(sizeof(engine_callback*) * initial_vecsize);  
}  

First of all, I have omitted some error checking as well as the code that reallocates the callback vector when the number of callbacks exceed the vector size.

However, when I run this code, the program crashes as described in the code. I'm guessing segmentation fault but I can't be sure since no output is given. I'm also guessing that the error comes from a somewhat flawed understanding on how to declare and allocate an array of pointers to function pointers.

Please Stack Overflow, guide me.

EDIT: I can't seem to grasp how to indent the code blocks. That's almost a tad embarassing...

EDIT: Nevermind that. Checked the page source of some other posts =).

Answer 1

At the line:

vec[vecsize++] = func; //error here!  

What happens if vecsize is >= initial_vecsize ?

Also __engine_allocate_ve doesn't work since it only modifies the local copy of vec, you have to change the signature to a ** and pass the argument with &.

static void __engine_allocate_vec(engine_callback** vec)

__engine_allocate_vec(&vec);

Answer 2

The allocation function should be:

static void __engine_allocate_vec(engine_callback** vec) {  
    *vec =  malloc(sizeof(engine_callback) * initial_vecsize);  
}  

and then:

if (!vec)  
    __engine_allocate_vec(&vec);  

Note that the pointer type mismatch in your original allocate function would have been caught if you had omitted the cast. also, don't use names containing double underscores in your code - they are meant for the implementation's use.

Answer 3

You seem to be malloc-ing based on sizeof(engine_callback*) rather than sizeof(engine_callback) ...

Answer 4

Your __engine_allocate_vec function is creating space for new engine_callbacks, but it's not doing anything useful with that pointer. (It's changing its local version of vec, which was passed by value -- so the changes don't make it back to the caller. And the argument's name hides the global's name, so that's not getting set either.) So when it returns, your pointer's still null.

Try something like this...

static void __engine_allocate_vec(engine_callback** vec) {
    *vec = (engine_callback*) malloc(sizeof(engine_callback) * initial_vecsize);
}

Then in register_event_callback, pass &vec to the function instead of vec.

Or, make the function void and let it set the global itself. Nah, forget i said that.

Answer 5

First of all, do not use leading underscores for variable or function names; such names are reserved for the implementation.

Everyone else has pointed out the right way to initially allocate the vector:

static void engine_allocate_vec(event_callback **vec)
{
  *vec = malloc(sizeof **vec * initial_vecsize);
}

Notice a couple of things. First of all, I do not cast the result of malloc. It isn't necessary (as of C89, anyway; earlier versions of C require a cast, as does C++), and it can potentially suppress a compiler diagnostic if you forget to include stdlib.h or otherwise don't have a prototype for malloc in scope. Second of all, I'm calling sizeof on the expression **vec, not the type; since the type of the expression **vec isevent_callback, sizeof **vec returns the same result as sizeof (event_callback). This helps cut down on visual clutter, and it avoids a somewhat common bug that creeps in when someone changes the type of the variable, but doesn't carry that change through to the sizeof expression in the malloc call, such as

double *f; /* was originally declared as float, later changed to double */
...
f = malloc(sizeof (float) * size); /* type change not carried through */

Note that sizeof does not evaluate its operand (unless it's a VLA), so you don't have to worry about calling it on an uninitialized pointer expression.

This helps you create the initial vector. However, you'd like to be able to extend the vector as necessary when registering more than initial_vecsize callbacks, right? If so, let me suggest the following:

static int engine_allocate_vec(event_callback **vec, 
  size_t *currentSize, 
  size_t extent)
{
  int success = 0;
  /**
   * Assign the result of realloc to a temporary; realloc returns NULL
   * on failure, and we don't want to risk losing our pointer to the
   * previously allocated memory.  Similarly, we don't update *currentSize
   * unless the call succeeds.  Note that realloc(NULL, size) is equivalent
   * to malloc(size).
   */
  event_callback *tmp = realloc(*vec, sizeof *tmp * (*currentSize + extent));
  if (tmp != NULL)
  {
    *vec = tmp;
    *currentSize += extent;
    success = 1;
  }
  return success;
}

Then your register function becomes:

/**
 * Adding vector_count variable to keep track of the number
 * of items in the vector as opposed to the physical size
 * of the vector.
 */
static size_t vector_count = 0;

void register_callback_event(event_callback func)
{
  if (!vec)
  {
    /**
     * Initial vector allocation
     */
    if (!engine_allocate_vec(&vec, &vecsize, initial_vecsize))
    {
      /* allocation failed; treating this as a fatal error */
      exit(0);
    }
  }
  else if (vector_count == vecsize)
  {
    /**
     * Need to extend the vector to accomodate 
     * the new callback.  Double the vector size (i.e.,
     * extend it by the current vector size)
     */
    if (!engine_allocate_vec(&vec, &vecsize, vecsize))
    {
      /* extension failed - treating this as a fatal error*/
      free(vec);
      exit(0);
    }
  }

  vec[vector_count++] = func;
}