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

Implementing pthread barrier for Mac OS/X

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

Problem

I have written this little thingie to fix a problem of missing pthread_barrier_t in Mac OS/X pthreads. Are there any issues with this code?

The header:

#ifndef PTHREAD_BARRIER_H
#define PTHREAD_BARRIER_H

#include 

#ifdef __APPLE__

#ifdef __cplusplus
extern "C" {
#endif

#if !defined(PTHREAD_BARRIER_SERIAL_THREAD)
# define PTHREAD_BARRIER_SERIAL_THREAD  (1)
#endif

#if !defined(PTHREAD_PROCESS_PRIVATE)
# define PTHREAD_PROCESS_PRIVATE    (42)
#endif
#if !defined(PTHREAD_PROCESS_SHARED)
# define PTHREAD_PROCESS_SHARED     (43)
#endif

typedef struct {
} pthread_barrierattr_t;

typedef struct {
    pthread_mutex_t mutex;
    pthread_cond_t cond;
    unsigned int limit;
    unsigned int count;
    unsigned int phase;
} pthread_barrier_t;

int pthread_barrierattr_init(pthread_barrierattr_t *attr);
int pthread_barrierattr_destroy(pthread_barrierattr_t *attr);

int pthread_barrierattr_getpshared(const pthread_barrierattr_t *restrict attr,
                   int *restrict pshared);
int pthread_barrierattr_setpshared(pthread_barrierattr_t *attr,
                   int pshared);

int pthread_barrier_init(pthread_barrier_t *restrict barrier,
             const pthread_barrierattr_t *restrict attr,
             unsigned int count);
int pthread_barrier_destroy(pthread_barrier_t *barrier);

int pthread_barrier_wait(pthread_barrier_t *barrier);

#ifdef  __cplusplus
}
#endif

#endif /* __APPLE__ */

#endif /* PTHREAD_BARRIER_H */


The source file:

```
#include "pthread_barrier.h"

#include

#ifdef __APPLE__

#define __unused __attribute__((unused))

int
pthread_barrierattr_init(pthread_barrierattr_t *attr __unused)
{
return 0;
}

int
pthread_barrierattr_destroy(pthread_barrierattr_t *attr __unused)
{
return 0;
}

int
pthread_barrierattr_getpshared(const pthread_barrierattr_t *restrict attr __unused,
int *restrict pshared)
{
*pshared = PTHREAD_PROCESS_PRIVATE;
return 0;
}

int
pthread_barrierattr_setpshared(pthread_barrierattr_t *a

Solution

No problems found

As they say, "no news is good news". I read your code all the way through with an eye towards finding any possible errors, but I didn't find anything to criticize.

Instead I'll point out the things I liked about your code.
Use of restrict keyword

I must admit, I don't use restrict but I should learn to, as it can only help the compiler. I already advocate using const when appropriate, and this is similar.
Proper error checking

You check for errors and set errno appropriately.
Correct barrier operation

I tried to find any concurrency flaw in your barrier wait function but it looked correct.
The one weird thing

The one thing I thought was strange was that you used a do {} while loop without the curly braces:

do
        pthread_cond_wait(&barrier->cond, &barrier->mutex);
    while (phase == barrier->phase);


I've never seen that before but I guess if you like to do that it's fine. I think people throw in the curlies because it doesn't waste any lines, whereas with a while loop or an if block it would add an extra line:

while (condition) {
        foo();
    }                       // <-- Wasted line

    do {
        foo();
    } while (condition);    // <-- No waste

Code Snippets

do
        pthread_cond_wait(&barrier->cond, &barrier->mutex);
    while (phase == barrier->phase);
while (condition) {
        foo();
    }                       // <-- Wasted line

    do {
        foo();
    } while (condition);    // <-- No waste

Context

StackExchange Code Review Q#88269, answer score: 3

Revisions (0)

No revisions yet.