Reference-counted smart pointer in C

Question

About

I've been experimenting with gcc's __cleanup__ attribute, and thought it'd be a great fit for a memory-safe smart pointer for C.

This is the implementation. It requires you to use either the helper macros SHARED_PTR_VAR_* or declare a variable as shared_ptr_t __attribute__((__cleanup__(cleanup_shared_ptr))) var={0};, and since it relies on gcc's extensions, it obviously requires a compiler that supports them.

The reference counting is thread-safe, but the pointer access itself is not synchronized.

shared_ptr.h

#pragma once

#include <pthread.h>
#include <stdint.h>
#include <stdlib.h>
#include "util.h"

typedef struct shared_ptr_cntrl {
    void * data;
    void (*cleanup)(void*);
    pthread_mutex_t mutex;
    int64_t count;
    int64_t cntrl_count;
} shared_ptr_cntrl_t;

typedef struct shared_ptr {
    shared_ptr_cntrl_t * cntrl;
} shared_ptr_t;

shared_ptr_t create_shared_ptr(void ** data,void (*cleanup)(void*));

void cleanup_shared_ptr(shared_ptr_t *);

shared_ptr_t copy_shared_ptr(shared_ptr_t *);

static inline shared_ptr_t move_shared_ptr(shared_ptr_t * p){
    shared_ptr_t tmp={p->cntrl};
    p->cntrl=NULL;
    return tmp;
}

static inline void * get_shared_ptr_ptr(shared_ptr_t * p){
    return (p->cntrl)?p->cntrl->data:NULL;
}

#define SHARED_PTR_FROM(type,cleanup,expr) ({\
    type * v=calloc(1,sizeof(type));\
    if(!v) err_exit("\n\nOUT OF MEMORY\n\n");\
    *v=expr;\
    create_shared_ptr((void**)&v,(void(*)(void*))cleanup);\
})

#define SHARED_PTR_FROM_E(cleanup,expr) SHARED_PTR_FROM(__typeof__((expr)),cleanup,expr)

#define SHARED_PTR_VAR_FROM_E(name,cleanup,expr) CLEANUP_VAR_E(cleanup_shared_ptr,name,SHARED_PTR_FROM_E(cleanup,expr))

#define SHARED_PTR_VAR_E(name,expr) CLEANUP_VAR_E(cleanup_shared_ptr,name,expr)

#define SHARED_PTR_VAR_FROM(type,name,cleanup,expr) CLEANUP_VAR_E(cleanup_shared_ptr,name,SHARED_PTR_FROM(type,cleanup,expr));

util.h

#define CLEANUP(cleanup) __attribute__((__cleanup__(cleanup)))
#define CLEANUP_VAR(type,cleanup,name) type __attribute__((__cleanup__(cleanup))) name
#define CLEANUP_VAR_E(cleanup,name,expr) CLEANUP_VAR(__typeof__(expr),cleanup,name) = (expr) ;

__attribute__((format(printf,1,2)))
__attribute__((__noreturn__)) void err_exit(const char * fmt,...);

shared_ptr.c

#include "shared_ptr.h"

shared_ptr_t create_shared_ptr(void ** data,void (*cleanup)(void*)){
    if(!*data){
        return (shared_ptr_t){NULL};//avoid allocation for NULL pointers
    }
    shared_ptr_cntrl_t * cntrl=calloc(1,sizeof(shared_ptr_cntrl_t));
    cntrl->data=*data;
    *data=NULL;
    cntrl->cleanup=cleanup;
    cntrl->count=1;
    cntrl->cntrl_count=1;
    if(pthread_mutex_init(&cntrl->mutex,NULL)){
        err_exit("Failed to create mutex for shared_ptr");
    }
    return (shared_ptr_t){cntrl};
}

void cleanup_shared_ptr(shared_ptr_t * p){
    if(p->cntrl){
        if(pthread_mutex_lock(&p->cntrl->mutex)){
            err_exit("Failed to lock mutex for shared_ptr");
        }
        p->cntrl->count--;
        p->cntrl->cntrl_count--;
        if(p->cntrl->count==0){
            if(p->cntrl->cleanup){
                p->cntrl->cleanup(p->cntrl->data);
            }
            free(p->cntrl->data);
            p->cntrl->data=NULL;
            if(p->cntrl->cntrl_count==0){
                pthread_mutex_t m=p->cntrl->mutex;
                free(p->cntrl);
                p->cntrl=NULL;
                if(pthread_mutex_unlock(&m)){
                    err_exit("Failed to unlock mutex for shared_ptr");
                }
                if(pthread_mutex_destroy(&m)){
                    err_exit("Failed to destroy mutex for shared_ptr");
                }
                return;
            }
        }
        if(pthread_mutex_unlock(&p->cntrl->mutex)){
            err_exit("Failed to unlock mutex for shared_ptr");
        }
    }
}

shared_ptr_t copy_shared_ptr(shared_ptr_t * p){
    if(p->cntrl){
        if(pthread_mutex_lock(&p->cntrl->mutex)){
            err_exit("Failed to lock mutex for shared_ptr");
        }
        p->cntrl->count++;
        p->cntrl->cntrl_count++;
        if(pthread_mutex_unlock(&p->cntrl->mutex)){
            err_exit("Failed to unlock mutex for shared_ptr");
        }
    }
    return (shared_ptr_t){p->cntrl};
}

util.c

#include "util.h"
#include <stdlib.h>
#include <stdio.h>
#include <stdarg.h>

void err_exit(const char * fmt,...){
    va_list arg;
    va_start(arg,fmt);
    vfprintf(stderr,fmt,arg);
    va_end(arg);
    exit(1);
}

Example Code

#include <stdio.h>
#include "shared_ptr.h"

static void example_destruct(volatile int * p){
    printf("int destruct %d\n",*p);
}

int main(){
    SHARED_PTR_VAR_FROM(volatile int,example_ptr,example_destruct,20);
    
    SHARED_PTR_VAR_E(example_ptr_2,copy_shared_ptr(&example_ptr));
    
    printf("%d\n",*(volatile int*)get_shared_ptr_ptr(&example_ptr_2));
    
    SHARED_PTR_VAR_E(example_ptr_3,move_shared_ptr(&example_ptr_2));
    
    (*(volatile int*)get_shared_ptr_ptr(&example_ptr_3))=23;
    
    {
        SHARED_PTR_VAR_E(example_ptr_4,copy_shared_ptr(&example_ptr));
        SHARED_PTR_VAR_E(example_ptr_5,copy_shared_ptr(&example_ptr));
        SHARED_PTR_VAR_E(example_ptr_6,copy_shared_ptr(&example_ptr));
        SHARED_PTR_VAR_E(example_ptr_7,copy_shared_ptr(&example_ptr));
        SHARED_PTR_VAR_E(example_ptr_8,copy_shared_ptr(&example_ptr));
    }
    
    printf("%d\n",*(volatile int*)get_shared_ptr_ptr(&example_ptr));
    
    (*(volatile int*)get_shared_ptr_ptr(&example_ptr_3))=40;
}

Answer 1

Tolerate NULL during clean-up

As free(NULL) is OK, consider a NULL test for clean-up

void cleanup_shared_ptr(shared_ptr_t * p){
  if (p) {
    ....
  }
}

Lack of documentation

*.h files deserve some overall comments. Consider users may not want to wade through the .c file code to deduce functionality.

Fixed width

I see no compelling reasons for int64_t here. Consider int. If concerned that int is insufficient, go for intmax_t or long long.

// int64_t count, cntrl_count;
int count, cntrl_count;

Missing guard

util.h lacks a #pragma once or the like.

Naming

Some names shared_ptr.h names begin with shared_ptr, others have shared_ptr in the middle. Recommend uniformity, IMO, begin with shared_ptr.

util.h is far too generic and gives no hint that it defines err_exit().

format() has a high surprising collision potential.

shared_ptr_t is a struct type, yet contains "ptr". Better to not call things that are not pointers as ptr, etc.

Surprising exits

Good to have error detection. SHARED_PTR_FROM and others can exit code. I'd find this too draconian for general use as this code is effectively defining error handle for its use. Do the smart pointer task, yet leave final error handling to the application.

Allocate by object

Rather that allocate by the type, consider allocating by the referenced object. Easier to code right, review and maintain.

// shared_ptr_cntrl_t * cntrl=calloc(1,sizeof(shared_ptr_cntrl_t));
shared_ptr_cntrl_t * cntrl = calloc(1, sizeof * cntrl);

Good use of #include "util.h"

Nice to have util.c first include "util.h". It is a good test of util.h sufficiency.

Answer 2

I don't think these can be negative:

int64_t count;
int64_t cntrl_count;

Consider an unsigned type (perhaps size_t is most appropriate?)

This cast and associated use are not portable:

create_shared_ptr((void**)&v,(void(*)(void*))cleanup);  

          p->cntrl->cleanup(p->cntrl->data);

Although function pointers can be cast to another function pointer type, calling through such a pointer without casting it back is Undefined Behaviour. Instead, require that the caller provides a function pointer of that accepts void*, as qsort() and bsearch() do.

type * v=calloc(1,sizeof(type));
shared_ptr_cntrl_t * cntrl=calloc(1,sizeof(shared_ptr_cntrl_t));

I'd normally write sizeof *v and sizeof *cntrl respectively. It's not a big deal here, but is a good practice where the pointer's declaration is far away.

I don't think it's meaningful to copy a pthread mutex like this:

            pthread_mutex_t m=p->cntrl->mutex;
            free(p->cntrl);
            p->cntrl=NULL;
            if(pthread_mutex_unlock(&m)){
                err_exit("Failed to unlock mutex for shared_ptr");
            }
            if(pthread_mutex_destroy(&m)){
                err_exit("Failed to destroy mutex for shared_ptr");
            }

However, if we're in that code, then we know that there's no other reference in play, so we can simply destroy the mutex we have, before the free():

    pthread_mutex_t *const m = &p->cntrl->mutex;

        ...

            if(pthread_mutex_unlock(m)){
                err_exit("Failed to unlock mutex for shared_ptr");
            }
            if(pthread_mutex_destroy(m)){
                err_exit("Failed to destroy mutex for shared_ptr");
            }
            free(p->cntrl);
            p->cntrl=NULL;