Synchronous events library

Question

I have implemented a small library that handles synchronous events with POSIX compliant threads. I oriented me on the already existing POSIX thread API. Here are the files I created:

ptevent.h

/*!
 *  @file           ptevent.h
 *  @copyright      < Pro Fusion , 2014 >
 *  @date           31.08.14
 *  @discussion
 *  Synchronous events for POSIX threads.
 *
 *  Copyright 2014 Pro Fusion. All rights reserved.
 */

#if !defined(_ptevent_h)
    #define _ptevent_h 1

    typedef struct _ptevent     *ptevent_t;

    int ptevent_init(ptevent_t *const event);
    int ptevent_send(ptevent_t const event, unsigned const flags);
    int ptevent_sendall(ptevent_t const event, unsigned const flags);
    int ptevent_recv(ptevent_t restrict const event, unsigned *restrict const flags, unsigned const readmask, unsigned const clearmask);
    int ptevent_destroy(ptevent_t event);

#endif // !defined(_ptevent_h)

ptevent.c

/*!
 *  @file           ptevent.c
 *  @copyright      < Pro Fusion , 2014 >
 *  @date           31.08.14
 *
 *  Copyright 2014 Pro Fusion. All rights reserved.
 */

#include "ptevent.h"        // ptevent_t
#include <pthread.h>        // pthread_*
#include <errno.h>          // E*
#include <stdlib.h>         // malloc
#include <stdbool.h>        // bool

struct _ptevent {
    pthread_mutex_t     mtx;
    pthread_cond_t      recv_cnd;
    pthread_cond_t      send_cnd;

    unsigned            flag;
};

int ptevent_init(ptevent_t *const event)
{
    if (event == NULL) return EINVAL;

    {
        ptevent_t _event;

        int ret = ENOMEM;

        _event = malloc(sizeof(* _event));

        if (_event == NULL) goto error_0;

        ret = pthread_mutex_init(&_event->mtx, NULL);

        if (ret != 0) goto error_1;

        ret = pthread_cond_init(&_event->recv_cnd, NULL);

        if (ret != 0) goto error_2;

        ret = pthread_cond_init(&_event->send_cnd, NULL);

        if (ret != 0) goto error_3;

        _event->flag    = 0;

        *event = _event;

        return 0;

        error_3:
        (void)pthread_cond_destroy(&_event->recv_cnd);

        error_2:
        (void)pthread_mutex_destroy(&_event->mtx);

        error_1:
        free(_event);

        error_0:
        return ret;
    }
}

static int _ptevent_send(ptevent_t const event, unsigned const flags, bool const broadcast)
{
    if (event == NULL) return EINVAL;

    {
        int ret;

        ret = pthread_mutex_lock(&event->mtx);

        if (ret != 0) return ret;

        while (event->flag & flags) {
            (void)pthread_cond_wait(&event->recv_cnd, &event->mtx);
        }

        event->flag |= flags;

        if (broadcast) {
            (void)pthread_cond_broadcast(&event->send_cnd);
        } else {
            (void)pthread_cond_signal(&event->send_cnd);
        }

        (void)pthread_mutex_unlock(&event->mtx);

        return 0;
    }
}

int ptevent_send(ptevent_t const event, unsigned const flags)
{
    return _ptevent_send(event, flags, false);
}

int ptevent_sendall(ptevent_t const event, unsigned const flags)
{
    return _ptevent_send(event, flags, true);
}

int ptevent_recv(ptevent_t restrict const event, unsigned *restrict const flags, unsigned const readmask, unsigned const clearmask)
{
    if (event == NULL) return EINVAL;

    {
        int ret;

        ret = pthread_mutex_lock(&event->mtx);

        if (ret != 0) return ret;

        while (!(event->flag & readmask)) {
            (void)pthread_cond_wait(&event->send_cnd, &event->mtx);
        }

        if (flags != NULL) *flags = event->flag & readmask;
        event->flag &= clearmask;

        (void)pthread_cond_broadcast(&event->recv_cnd);
        (void)pthread_mutex_unlock(&event->mtx);

        return 0;
    }
}

int ptevent_destroy(ptevent_t const event)
{
    if (event == NULL) return EINVAL;

    (void)pthread_mutex_destroy(&event->mtx);
    (void)pthread_cond_destroy(&event->send_cnd);
    (void)pthread_cond_destroy(&event->recv_cnd);

    free(event);

    return 0;
}

test.c

#include <stdio.h>
#include "ptevent.h"
#include <pthread.h>
#include <unistd.h>
#include <assert.h>

ptevent_t events;

void *fcn(void *ctx)
{
    unsigned flags  = 0;

    for (;;) {
        assert(ptevent_recv(events, &flags, 0xFF, 0x00) == 0);

        if (flags & 0x01) {
            fprintf(stderr, "got event 1\n");
            sleep(1);
        }

        if (flags & 0x02) {
            fprintf(stderr, "got event 2\n");
        }

        if (flags & 0x04) break;
    }

    return NULL;
}

int main(int argc, const char * argv[])
{
    pthread_t thread;

    assert(ptevent_init(&events) == 0);

    assert(pthread_create(&thread, NULL, fcn, NULL) == 0);

    assert(ptevent_send(events, 0x01) == 0);
    assert(ptevent_send(events, 0x02) == 0);
    assert(ptevent_send(events, 0x03) == 0);
    assert(ptevent_send(events, 0x04) == 0);

    assert(pthread_join(thread, NULL) == 0);

    return 0;
}

Answer 1

It's common practice to make macro names upper case. If you choose to make your include guard upper case you should remove the leading underscore, as names beginning with an underscore followed by an upper case letter are reserved (GNU manual).

#ifndef PTEVENT_H
#define PTEVENT_H

---

In ptevent_recv, _ptevent_send and ptevent_init you use a peculiar pattern. You return inside an if, so you don't bother to create an else clause, but next you want to do some declarations. The result is a block statement right after an if statement which is a bit confusing.

if (event == NULL) return EINVAL;

{
    // Declarations

A more idiomatic way would be to make the block statement into an else clause.

if (event == NULL) {
    return EINVAL;
}
else {
    // Declarations

---

In ptevent_init you've put a newline between almost every statement except for the if. I think you could reformat the code for greater readability.

ret = pthread_mutex_init(&_event->mtx, NULL);
if (ret != 0) {
    goto error_1;
}

ret = pthread_cond_init(&_event->recv_cnd, NULL);
if (ret != 0) {
    goto error_2;
}

ret = pthread_cond_init(&_event->send_cnd, NULL);
if (ret != 0) {
    goto error_3;
}

Multiple statements per line, like below, is usually bad practice since you can't put a breakpoint on the latter statements.

if (event == NULL) return EINVAL;

---

You can shorten your code and connect coupled code by keeping variable declaration and assignment on the same line.

int ret = pthread_mutex_lock(&event->mtx);