timer struct for fixed timestep iterations in C (gcc)

Question

I was toying with a way force fixed timestep loop iterations in C and came up with a fairly simple timer struct (quite different from Pausable Timer Implementation for SDL in C and decided to put it out for review. The timerstruct itself is:

typedef struct ktimer_t {
    struct timespec next;               /* timespec for next iteration */
    double tstep;                       /* seconds per time-step */
    void (*keep) (struct ktimer_t *);   /* keep time function */
    void (*reset) (struct ktimer_t *, double);  /* reset tstep & next */
} ktimer_t;

Where the keep (keep_time) function grabs the current time with calculates clock_gettime() and then computes the difference between the next timestep (held in .next) and then calls nanosleep() to sleep for that period of time.

The timer is initialized with the number of seconds (or fractions of a second) in for each timestep, assigns the member function addresses for keep and reset, stores the current time and adds the timestep to that for use computing the difference when keep is called. The reset function simply allows reset of the time to current and resetting the timestep to a different value allowing for timer reuse. The init and member functions are:

/** takes pointer to ktimer_t contianing next timestep, subtracts
 *  current time from next and nanosleeps for the difference
 *  updating k->next to contain the next timestep.
 */
void ktimer_keep (ktimer_t *k)
{
    struct timespec req = { .tv_sec = 0 }, rem = { .tv_sec = 0 };

    clock_gettime (CLOCK_REALTIME, &rem);   /* get current time */
    req = tsdiff (&k->next, &rem);          /* time to next timestep */

    /* validate non-negative time difference */
    if ((double)req.tv_sec < 0 || (double)req.tv_nsec < 0) {
        fputs ("error: next timestep is in the past.\n", stderr);
        return; /* or exit(EXIT_FAILURE) as desired */
    }
    else
        nanosleep (&req, &rem); /* sleep until requested time */

    set_nextstep (k);   /* update timespec to next */
}

/** resets ktimer_t current time and initializes
 *  .next for the next timestep.
 */
void ktimer_reset (ktimer_t *k, double tstep)
{
    clock_gettime (CLOCK_REALTIME, &k->next);   /* get current time */
    k->tstep = tstep;               /* set timestep */
    set_nextstep (k);               /* update timespec to next */
}

/** declares and initializes a ktimer_t struct.
 *  setting .tstep timestep to tstep, assigns the
 *  addresses of member functions, and updates .next
 *  to current time + k->tstep with set_nextstep().
 */
ktimer_t ktimer_init (double tstep)
{
    ktimer_t k = {  .tstep = tstep,
                    .keep = ktimer_keep, 
                    .reset = ktimer_reset };

    clock_gettime (CLOCK_REALTIME, &k.next);    /* get current time */
    set_nextstep (&k);                  /* update timespec to next */

    return k;
}

The two other helper functions are set_nextstep() and tsdiff(). set_nextstep just adds another timestep to the .next timespec keeping a fixed interval (aside from any negligible rounding error from the double addition). The tsdiff function computes the sleep time required to keep the iterations on a fixed time, e.g.

/** simple helper to update timespec to next timestep
 *  (repetitive code)
 */
static void set_nextstep (ktimer_t *k)
{
    if (k->tstep >= 1.0)    /* validate if adding whole or fractional secs */
        k->next.tv_sec += k->tstep; /* add number of seconds to next */
    else    /* or nanoseconds */
        k->next.tv_nsec += k->tstep * 1e9;    
}

/** tsdiff returnds a struct timespec with difference between tend/tbeg.
 *  tend must hold a timespec later in time than tbeg.
 */
struct timespec tsdiff (struct timespec *tend, struct timespec *tbeg)
{
    struct timespec diff = { .tv_sec = 0 }; /* timespec to hold difference */
    double sec = tend->tv_sec + 1e-9 * tend->tv_nsec -  /* diff in seconds */
                (tbeg->tv_sec + 1e-9 * tbeg->tv_nsec);

    diff.tv_sec = (unsigned long)sec;                   /* set diff seconds */
    diff.tv_nsec = (sec - (unsigned long)sec) * 1e9;    /* set nanoseconds */

    return diff;    /* return timespec holding difference */
}

For use, a timer struct is either declared and initialized (or .reset) immediately before entering the loop to be timed, and then the .keep time function is called at the end of the loop invoking nanosleep for the difference between the time it took to process all commands within the loop and the .next timespec time. Example:

    ktimer_t k = ktimer_init (step);    /* declare/initialize timer */

    for (double i = 0; i < SECS; i += step) {   /* loop seconds by timestep */
        printf ("%5.2f\n", i);          /* some long calculation here */
        k.keep(&k);                     /* nanosleep time to next timestep */
    }

My question here is two-fold (1) is this a reasonable factorization of the timekeeping having the struct store the .next timespec and then adding a fixed timestep each iteration, or (2) would what I called "negligible rounding error" from double addition the timestep each time be worse than keeping the begin time in the struct and passing iteration * timestep as the time since the beginning of the loop? (it seems like a wash, but I welcome your thoughts).

An additional question I have, is there any problem passing the pointer to struct itself as a parameter to the member function -- that then updates the member values of the struct itself? I can't find any part of the standard that says that's wrong, and for practical purposes the member functions are just functions, so if they happen to take a pointer to the struct as a parameter and update part of the struct, it doesn't really matter where the called function address comes from... (am I missing anything there?)

Here is a short example that exercises each part of the timer scheme:

#define _GNU_SOURCE
#include <stdio.h>
#include <time.h>

/* define SECS and TSTEP in compile string as desired with -D */
#ifndef SECS
#define SECS 10     /* number of realtime seconds to run */
#endif

#ifndef TSTEP
#define TSTEP 0.5   /* handles either whole or fractional timesteps */
#endif

typedef struct ktimer_t {
    struct timespec next;               /* timespec for next iteration */
    double tstep;                       /* seconds per time-step */
    void (*keep) (struct ktimer_t *);   /* keep time function */
    void (*reset) (struct ktimer_t *, double);  /* reset tstep & next */
} ktimer_t;

/** simple helper to update timespec to next timestep
 *  (repetitive code)
 */
static void set_nextstep (ktimer_t *k)
{
    if (k->tstep >= 1.0)    /* validate if adding whole or fractional secs */
        k->next.tv_sec += k->tstep; /* add number of seconds to next */
    else    /* or nanoseconds */
        k->next.tv_nsec += k->tstep * 1e9;    
}

/** tsdiff returnds a struct timespec with difference between tend/tbeg.
 *  tend must hold a timespec later in time than tbeg.
 */
struct timespec tsdiff (struct timespec *tend, struct timespec *tbeg)
{
    struct timespec diff = { .tv_sec = 0 }; /* timespec to hold difference */
    double sec = tend->tv_sec + 1e-9 * tend->tv_nsec -  /* diff in seconds */
                (tbeg->tv_sec + 1e-9 * tbeg->tv_nsec);

    diff.tv_sec = (unsigned long)sec;                   /* set diff seconds */
    diff.tv_nsec = (sec - (unsigned long)sec) * 1e9;    /* set nanoseconds */

    return diff;    /* return timespec holding difference */
}

/** takes pointer to ktimer_t contianing next timestep, subtracts
 *  current time from next and nanosleeps for the difference
 *  updating k->next to contain the next timestep.
 */
void ktimer_keep (ktimer_t *k)
{
    struct timespec req = { .tv_sec = 0 }, rem = { .tv_sec = 0 };

    clock_gettime (CLOCK_REALTIME, &rem);   /* get current time */
    req = tsdiff (&k->next, &rem);          /* time to next timestep */

    /* validate non-negative time difference */
    if ((double)req.tv_sec < 0 || (double)req.tv_nsec < 0) {
        fputs ("error: next timestep is in the past.\n", stderr);
        return; /* or exit(EXIT_FAILURE) as desired */
    }
    else
        nanosleep (&req, &rem); /* sleep until requested time */

    set_nextstep (k);   /* update timespec to next */
}

/** resets ktimer_t current time and initializes
 *  .next for the next timestep.
 */
void ktimer_reset (ktimer_t *k, double tstep)
{
    clock_gettime (CLOCK_REALTIME, &k->next);   /* get current time */
    k->tstep = tstep;               /* set timestep */
    set_nextstep (k);               /* update timespec to next */
}

/** declares and initializes a ktimer_t struct.
 *  setting .tstep timestep to tstep, assigns the
 *  addresses of member functions, and updates .next
 *  to current time + k->tstep with set_nextstep().
 */
ktimer_t ktimer_init (double tstep)
{
    ktimer_t k = {  .tstep = tstep,
                    .keep = ktimer_keep, 
                    .reset = ktimer_reset };

    clock_gettime (CLOCK_REALTIME, &k.next);    /* get current time */
    set_nextstep (&k);                  /* update timespec to next */

    return k;
}

int main (void) {

    double step = TSTEP;
    ktimer_t k = ktimer_init (step);    /* declare/initialize timer */

    for (double i = 0; i < SECS; i += step) {   /* loop seconds by timestep */
        printf ("%5.2f\n", i);          /* some long calculation here */
        k.keep(&k);                     /* nanosleep time to next timestep */
    }
    putchar ('\n');

    step /= 2.;
    k.reset (&k, step);   /* reset timer & tstep, repeat */
    for (double i = 0; i < SECS; i += step) {
        printf ("%5.2f\n", i);
        k.keep(&k);
    }
}

For the purposes of this example, I'm not overly concerned with whether CLOCK_REALTIME or CLOCK_MONOTONIC_RAW, etc.. would be best, though CLOCK_MONOTONIC_RAW would be the better choice. For the example and whether this was sane, the potential for discontinuous jumps in system time were not factors. I'm more concerned about whether this is a reasonable approach and if I'm missing anything obvious in the way I'm passing a pointer to the struct itself to a member function within the struct.

note: Windows QueryPerformanceCounter() should be able to provide similar functionality on that OS in the absence of clock_gettime().

Answer 1

- error handling

int nanosleep(const struct timespec *req, struct timespec *rem);
int clock_gettime(clockid_t clk_id, struct timespec *tp);

Return an error code. You may want to react to it or not, but probably you should let the user know, so I would change these:

int ktimer_keep(ktimer_t *k);
int ktimer_reset(ktimer_t *k, double tstep);
int ktimer_init(ktimer_t *k, double tstep)

You also may (or may not) want to force the user to read the error code (GCC extension):

int ktimer_keep(ktimer_t *k) __attribute__((warn_unused_result));

Documentation here.

---

- types

struct timespec::tv_sec is of type time_t, not unsigned long

struct timespec::tv_nsec is of type long, not unsigned long

---

- precision

double is typically 64 bits (it can't represent all 64-bit integers), long is also typically 64 bits, but long double is typically larger than 64 bits (it's implementation defined, but it's more or less stable) and usually can represent all 64-bit integers, so maybe it would be a better type.

Remember to use the correct constants if you do change to this type: 1e-9L I think it is.

---

- Unnecessary else (From Linux checkpatch.pl)

"else is not generally useful after a break or return."

I add to that sentence a continue, goto or exit().

Example:

if (err)
        return;
else
        printf("Hello world!\n");

is equivalent to this, which is easier to read:

if (err)
        return;
printf("Hello world!\n");

---

EDIT: continue revision

---

In your case, I would use goto to move all the error handling to the end of the function, and let the error-free path clear (this is opinion based; feel free to disagree):

int ktimer_keep(ktimer_t *k)
{
        struct timespec req = { .tv_sec = 0 };
        struct timespec rem = { .tv_sec = 0 };

        if (clock_gettime(CLOCK_REALTIME, &rem))
                goto err_lib;
        req = tsdiff (&k->next, &rem);

        if (req.tv_sec < 0 || req.tv_nsec < 0)
                goto err_past;
        if (nanosleep(&req, &rem))
                goto err_lib;

        set_nextstep(k);
        return 0;
err_past:
        fputs("error: next timestep is in the past.\n", stderr);
        return -1;
err_lib:
        perror("Write something meaningful here");
        // Maybe some more cleanup here
        return errno;
}

---

- casts (unneeded?)

if ((double)req.tv_sec < 0 || (double)req.tv_nsec < 0)

I don't see why you would need to cast that

---

- Warn user if misusing the function

Given that you are using GCC, you can ensure that the user doesn't shoot himself in the foot passing a NULL pointer to your functions. You can do that with

int ktimer_keep (ktimer_t *k) __attribute__((nonnull(1)));

Documentation here.