"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

• tofro: 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
• Conor O'Brien: @tofro hah, that's a good point... D'oh :p
• Toby Speight: What's "standard" about camelCase? I don't think there's any camelCase in namespace std...
• Toby Speight: 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!
• Conor O'Brien: @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?

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.

• Conor O'Brien: I used STATUS as a specifier. You would suggest changing the specifier to ERROR, so wouldn't ERROR_GENERIC be a better name?
• chux: @CᴏɴᴏʀO'Bʀɪᴇɴ Yes.

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(....);
  

• Deduplicator: The program is done in on allocation failure, so not freeing memory would be immaterial anyway.
• vnp: @Deduplicator Agreed. I felt that quitting program in this case is too cruel, and didn't pay proper attention. Fixed
• Conor O'Brien: 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.
• vnp: @CᴏɴᴏʀO'Bʀɪᴇɴ No, there is no mandate. It is just more consistent. Consider however a declaration int x, *y;.
• chux: @CᴏɴᴏʀO'Bʀɪᴇɴ Concerning --> * vs. -->* : the C spec uses void *malloc(size_t size);