HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Timer framework in C

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
timerframeworkstackoverflow

Problem

I am trying to create a very simple timer framework which allows you to setup event handling based on a timeout.

The basic primitives allow the programmer to:

  • allocate or free timers



  • arm/disarm timers



  • set attributes and timeout for a timer



Currently, the self contained example will create timers up to the maximum (currently defined as 10) with random timeout values from up to 20 seconds. Each timer will eventually expire and the associated call-back function executed.

```
/ Very simplistic timer framework by amallory@qnx.com /
#include
#include
#include
#include #include
#include
#include

#define NUM_TIMERS 10
#define MAX_RANDOM_TIME_MS 20000

enum timer_callback_retval {
CB_RETURN_NORMAL = 0,
CB_RETURN_FREE_TIMER,
CB_RETURN_INVALID,
};

enum timer_type {
TT_RELATIVE = 0,
TT_ABSOLUTE,
TT_INVALID,
};

/ Define our timer list type /
TAILQ_HEAD(timer_list, timer_node);

/ Master tick clock/count /
uint64_t tick_cnt = 0;

/*
Timer data structure:
-the linked list
-the monotonic fire time (saved as an absolute time)
-the user callback handler to run on expiry of timer
-the registered data pointer to pass to the user callback
*/
struct timer_node {
TAILQ_ENTRY(timer_node) entries;
uint64_t fire;
int (cb)(void user_data);
void *user_data;
};

/ Our global timer lists /
struct timer_list active_timers;
struct timer_list free_timers;
struct timer_node *timer_memory;

#ifdef DEBUG
/ print out the contents of a given timer list /
void print_list(struct timer_list *list) {
struct timer_node *np;
TAILQ_FOREACH(np, list, entries) {
printf("timer fire %llu\n", np->fire);
}
}
#endif

/ put a timer onto the free list /
static void free_timer(struct timer_node *timer) {
TAILQ_INSERT_HEAD(&free_timers, timer, entries);
}

/ pull an available timer off the free list /
static struct timer_node* alloc_timer(void) {
struct timer_node *np;
if(TAILQ_EMPTY(&free_timers)) return NULL;

Solution

Usability could be improved a bit. For example, alloc_timer() (note, name is deceptive as no actual allocation takes place) could be put inside of the set_timer() function. Let your framework worry about finding a free timer in the pool, or returning an error code. A separate function for allocating the timer would be more useful if you weren't using a pool and if it was important for a user to control when memory allocations occur.

I don't like how the callback method controls whether the timer is freed. I could see lots of potential timer 'leaks'. I think if you added a repeating timer type, most of the cases where you wouldn't want to free your timer could then be taken care of by your framework. If you really need a handle to a timer that you could guarantee would be available when you need it, I think extending or overloading alloc_timer() (and/or set_timer()) would be a better place. That way, the decision to take it off and put it back on the free_timers list is in the same line of code.

In clock_tick(), you're removing an element off the active_timers list while you iterate over it. In your case it looks like it'll work but that's usually an unsafe thing to do, I'd double check that.

Context

StackExchange Code Review Q#7406, answer score: 4

Revisions (0)

No revisions yet.