"Safe" memory management

Question

In order to concisify the handling of a failed call to malloc, realloc, or calloc, I have made a few files to dictate "safe" memory management. This is part of my project stac, so the prefix will appear in the following code snippets. Further, I have made the decision to use snake_case as opposed to standard camelCase.

What I hope to get out of this, and questions I have: are these functions in bad practice? Is there a different way I should be handling failed calls to memory allocators? And is there anything particularly wrong with the code I have written?

safe_mem.c

#include "safe_mem.h"

void* safe_malloc(size_t size){
    void* memory = malloc(size);

    if(memory == NULL){
        free(memory);
        runtime_error("failed to allocate memory.");
    }

    return memory;
}

void* safe_calloc(size_t num, size_t size){
    void* memory = calloc(num, size);

    if(memory == NULL){
        free(memory);
        runtime_error("failed to allocate memory.");
    }

    return memory;
}

void* safe_realloc(void* ptr, size_t size){
    void* memory = realloc(ptr, size);

    if(memory == NULL){
        free(memory);
        runtime_error("failed to reallocate memory.");
    }

    return memory;
}

safe_mem.h

#ifndef STAC_SAFE_MEM
#define STAC_SAFE_MEM
#include <stdio.h>
#include <stdlib.h>
#include "error.h"

void* safe_malloc(size_t);
void* safe_calloc(size_t, size_t);
void* safe_realloc(void*, size_t);
#endif

error.c

#include "error.h"

void make_error(char* type, char* message){
    eprintf("%s error: %s\n", type, message);
}

void fatal_error(char* type, char* message, int exit_code){
    make_error(type, message);
    exit(exit_code);
}

void runtime_error(char* message){
    fatal_error("Runtime", message, STATUS_RUNTIME_ERROR);
}

void generic_error(char* message){
    fatal_error("Generic", message, STATUS_GENERIC_ERROR);
}

error.h

#ifndef STAC_ERROR
#define STAC_ERROR
#include <stdio.h>
#include <stdlib.h>

#define eprintf(...) fprintf(stderr, __VA_ARGS__)
#define STATUS_OKAY (0)
#define STATUS_RUNTIME_ERROR (1)
#define STATUS_GENERIC_ERROR (-1)

void make_error(char*, char*);
void fatal_error(char*, char*, int);

void runtime_error(char*);
void generic_error(char*);

#endif

Answer 1

are these functions in bad practice?

I would say not, as long as the original functions they are wrapping have the same function signature to allow their replacement.

is there anything particularly wrong with the code I have written?

Allocating 0 bytes may return NULL, which does not indicate Out-of-memory.
Same for calloc(), realloc().

void* memory = malloc(size);
// if(memory == NULL){
if(memory == NULL && size > 0){

No point in calling free(memory) after if(memory == NULL)

if(memory == NULL){
  //  free(memory);

With eprintf(...), consider passing __FUNC__, __LINE__ of the error source to aid in debugging. Or at least create unique error messages.

void* safe_malloc(size_t size){
   runtime_error("failed to malloc memory.");

void* safe_calloc(size_t num, size_t size){
    runtime_error("failed to calloc memory.");

In a couple places, use const to allow wider application of the functions that are not changing the referenced data.

// void make_error(char* type, char* message){
void make_error(const char* type, const char* message){

I am not a fan of your name-space choices for error.h which includes eprintf, STATUS_OKAY, STATUS_GENERIC_ERROR, make_error, ... If I say these used in other ..c files how would I be guided back to error.h/error.c as their origin? Although a bit verbose, I prefer error_printf, ERROR_STATUS_GENERIC,...

Minor: #include <stdio.h> need not be in safe_mem.h nor #include <stdlib.h> in error.h. error.c should have #include <stdlib.h> as it calls exit(). error.c should not rely on error.h including that. Similar issues for safe_mem.c

---

From a design standpoint, I would also include void safe_free(void *) as the functionality of these safe functions may expand and require that even if it present does nothing. It makes safe_...() symmetric.

Answer 2

• Only #include what you need. safe_mem.h doesn't require any file it includes. On the other hand, safe_mem.c does require error.h and stdlib.h.

  Notice that where stdlib.h is included is important. The client shouldn't care whether safe_mem.c implementation depends on stdlib.h or not. If it is exposed through the safe_mem.h, then changing safe_mem.c may require you to fix safe_mem.h and recompile anything which depends on it.

  Along the same line, I recommend to move #include <stdio.h> #define eprintf to error.c (unless there is a compelling reason to export this name), and drop #include <stdlib.h>.

• safe_realloc is not really safe, because it will lead to memory leaks on realloc failure. Please disregard.

• Bind * to the variable, not to the type:

  void *safe_malloc(....);