# “Safe” memory management

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

• I don't see the point in freeing memory that couldn't be allocated in all your allocation calls - Although freeing a NULL pointer doesn't normally hurt, it also doesn't do anything – tofro Jun 22 '17 at 7:24
• @tofro hah, that's a good point... D'oh :p – Conor O'Brien Jun 22 '17 at 7:25
• What's "standard" about camelCase? I don't think there's any camelCase in namespace std... – Toby Speight Jun 23 '17 at 12:54
• I dislike the name "safe" - that's only the case if exiting the process is somehow safer than checking for NULL - and that's patently not the case for many programs. malloc_or_die() (and similar) would be much more honest! – Toby Speight Jun 23 '17 at 12:56
• @TobySpeight I've never looked deeply into case convention in C, but I rather assumed it to be like C++. Apparently not. As for your second point, I don't see any alternative other than exiting the program. How could one recover from a memory shortage in a program? – Conor O'Brien Jun 23 '17 at 17:34

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.

• I used STATUS as a specifier. You would suggest changing the specifier to ERROR, so wouldn't ERROR_GENERIC be a better name? – Conor O'Brien Jun 23 '17 at 0:52
• @CᴏɴᴏʀO'Bʀɪᴇɴ Yes. – chux Jun 26 '17 at 23:50
• 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(....);

• The program is done in on allocation failure, so not freeing memory would be immaterial anyway. – Deduplicator Jun 22 '17 at 0:32
• @Deduplicator Agreed. I felt that quitting program in this case is too cruel, and didn't pay proper attention. Fixed – vnp Jun 22 '17 at 0:34
• About your last point, I've seen it both ways. Is this a strict mandate? I personally find it more intuitive that the * is bound to the type rather than the variable. – Conor O'Brien Jun 22 '17 at 1:27
• @CᴏɴᴏʀO'Bʀɪᴇɴ No, there is no mandate. It is just more consistent. Consider however a declaration int x, *y;. – vnp Jun 22 '17 at 2:03
• @CᴏɴᴏʀO'Bʀɪᴇɴ Concerning --> * vs. -->* : the C spec uses void *malloc(size_t size); – chux Jun 22 '17 at 22:11