0

I'm doing this callback on linux timer, but I don't know why the address changes when it was converted back on the callback function. Code below

typedef void* timer_cb_args;
typedef void (*timer_cb)(timer_cb_args);


struct cb_wrapper
{
    timer_cb callback;
    timer_cb_args args;
};

void callback_wrapper(union sigval sv)
{
    struct cb_wrapper *cb = (struct cb_wrapper*)(sv.sival_ptr);

    printf("Casted sival_ptr pointer on Callback wrapper: %p\n\n", cb);
    printf("Callback wrapper function pointer: %p\n", cb->callback);
    printf("Callback wrapper args pointer: %p\n\n", &cb->args);

    cb->callback(cb->args);
}

int timer_start(timer_handle_t *timer_handle,
                           timer_cb callback,
                           timer_cb_args args,
                           guint32 duration)
{
    int ret = 0;
    timer_t *timer = calloc(1, sizeof(timer_t));

    *timer_handle = (timer_handle_t) calloc(1, sizeof(timer_handle_t));
    (*timer_handle)->m_timer = timer;

    struct sigevent evp;
    memset(&evp, 0, sizeof(struct sigevent));

    struct cb_wrapper cbargs;
    memset(&cbargs, 0, sizeof(struct cb_wrapper));
    cbargs.callback = callback;
    cbargs.args = args;

    evp.sigev_notify = SIGEV_THREAD;
    evp.sigev_notify_function = &callback_wrapper;
    evp.sigev_value.sival_ptr = &cbargs;

    printf("sival_ptr pointer on call: %p\n",  evp.sigev_value.sival_ptr);
    printf("Function pointer: %p\n", cbargs.callback);
    printf("Args pointer on call: %p\n\n", cbargs.args);

    int timer_result;
    timer_result = timer_create(CLOCK_REALTIME, &evp, timer);

    if (timer_result < 0)
        return -1;

    struct itimerspec timespec;
    memset(&timespec, 0, sizeof(struct itimerspec));
    timespec.it_value.tv_sec = duration;

    timer_result = timer_settime(*timer, 0, &timespec, NULL);

    if (timer_result < 0)
        return -1;

    return ret;
}

output is:

sival_ptr pointer on call: 0x7ffce75c3950
Function pointer: 0x55f26d13abb4
Args pointer on call: 0x7ffce75c3a00
Callback wrapper.
Casted sival_ptr pointer on Callback wrapper: 0x7ffce75c3950 //OK same
Callback wrapper function pointer: 0x55f26d13abb4 //OK same
Callback wrapper args pointer: 0x7ffce75c3958 //NOK not same as above
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
dhon2407
  • 5
  • 2
  • 1
    `struct cb_wrapper cbargs;` That is a local variable that only has lifetime within function. You must not keep a reference to it beyond that point. Use memory that has longer lifetime - static variable or dynamically allocated memory or memory provided by the caller that exists for the required lifetime. – kaylum Jul 08 '22 at 04:15
  • This confuses me, that the function pointer is correct but the argument pointer is not and they are both inside the cb_wrapper cbards. – dhon2407 Jul 08 '22 at 04:26
  • Broken clock is right twice a day. That doesn't mean you can use a broken clock to tell time. You're doing something wrong. You got lucky and got what you wanted some of the time. But that's not relevant. – ikegami Jul 08 '22 at 04:27
  • It's Undefined Behaviour. Not worth trying to understand the exact result as it depends on the compiler, compiler options, other code executed before or after it, etc. And it can change at any point. – kaylum Jul 08 '22 at 04:27
  • 1
    The main problem is that you've made yourself confused by hiding numerous pointers behind typedef. Simply don't do that, ever. – Lundin Jul 08 '22 at 06:29

1 Answers1

0

The problem is here:

typedef void* timer_cb_args;
typedef void (*timer_cb)(timer_cb_args);

You are hiding pointers behind typedef and the only thing achieved by that is making everyone including yourself confused.

Therefore you write bugs such as this:

evp.sigev_notify_function = &callback_wrapper;
evp.sigev_value.sival_ptr = &cbargs;

callback_wrapper is a function pointer and they have special rules about dereferencing/decay (Why do function pointer definitions work with any number of ampersands '&' or asterisks '*'?), so that line works by accident.

cbargs is however just an ordinary void* so evp.sigev_value.sival_ptr = &cbargs; assigned a void** to a void*. And since evp.sigev_value.sival_ptr is a void*, that's allowed without the compiler giving diagnostic messages.

This is a really subtle bug! I managed to find it in some five minutes here, but it could as well have taken forever. And the root cause is bad typedef practices.

Fix it like this:

typedef void timer_cb (timer_cb_args);


struct cb_wrapper
{
    timer_cb* callback;
    void* args;
};

...

evp.sigev_notify_function = callback_wrapper;
evp.sigev_value.sival_ptr = cbargs;

And then clean up the rest of the code accordingly, clearing out all pointers hidden behind typedefs.


Also unrelated to this bug, as someone pointed out it isn't a good idea to pass local variables by reference to callbacks. Because once the function setting up the callback is done, that memory is toast. A normal fix when for example passing variables to thread callbacks, is to pass a pointer to dynamic memory. Or alternatively just ensure that the thread creating thread doesn't die/go out of scope before the end of execution, after all other threads are cleaned up.

Lundin
  • 195,001
  • 40
  • 254
  • 396