Problem with error checking for every malloc call?-CodePudding

I'm writing a simple utility in C and I'm writing codes to print error messages in STDERR.

I have a struct, defined as:

struct arguments
{
    FILE *source;
    int modifier_value;
    int filesize;
};

I have declared a pointer to the above struct, and allocated memory to it:

struct arguments *arg = NULL;
arg = malloc(sizeof(struct arguments));
if(arg == NULL)
{
    fprintf(stderr, "error: malloc - %s\n", strerror(errno));  //I know i could use perror as well, but I like the convention of using fprintf() for both stdout, stderr, and other file streams, just for symmetry
    return EXIT_FAILURE;
}

As you can see, I have only allocated memory sufficient to store one object of type struct arguments and I still error checking for it.

The problem is I have many pointers like that which points to space of one object, and error checking all of them just increases number of codes and affects readability of code.

Is it a "good practice" / "is it ok" if I ignore error checking just for the reason that I'm not allocating memory too much memory (I heard something about paging and I think system combines many pages if I request for too much memory and chances of error in that case would be high, but not for memory request of something like 64 bytes).

CodePudding user response：

Is it a "good practice" / "is it ok" if I ignore error checking just for the reason that I'm not allocating memory too much memory

It is poor practice to fail to error-check function calls that report on errors, except where you don't care whether the call succeeded. And you always care whether malloc() succeeds, or else you shouldn't be calling it in the first place. You don't know whether you are allocating too much memory unless you check whether your malloc() calls succeed.

The problem is I have many pointers like that which points to space of one object, and error checking all of them just increases number of codes and affects readability of code.

In the first place, use dynamic allocation only where you actually need it. Some people seem to have the idea that they need dynamic allocation wherever they want a pointer to an object. This absolutely is not the case. You need pointers if you are performing dynamic allocation, but you don't necessarily need dynamic allocation where you use pointers. Very often, static or automatic allocation can be combined with the address-of operator (unary &) instead. For example:

{
    struct arguments arg = {0};
    init_arguments(&arg);
    do_something(&arg);
    // all done with arg
}

You need dynamic allocation only when (i) you do not know at compile time how much memory you will need, or (ii) you need an object whose lifetime extends past the termination of the innermost block enclosing its creation.

When you really do need dynamic allocation, you can reduce the amount of boilerplate code by using a macro or a wrapper function. Similar applies to performing other success checks. For example, use a function such as this instead of using malloc() directly:

void *checked_malloc(size_t size) {
    void *result = malloc(size);

    if (result == NULL && size != 0) {
        fputs("error: malloc failed -- aborting...\n", stderr);
        abort();
    }
    return result;
}

CodePudding user response：

Create allocation wrapper functions to make always checking allocation success easy.

To expand on @Weather Vane idea of passing an argument to identify the caller, yet do it in a macro wrapper.

my_malloc.h

#ifndef MY_MALLOC
#define MY_MALLOC(size) my_malloc((size), __FUNC__, __LINE__)
#include <stdlib.h>

// Return a valid allocation or don't return.
void *my_malloc(size_t size, const char *func, unsigned line);

#endif

my_malloc.c

#include "my_maloc.h"

#include <stdio.h>
#include <stdlib.h>

void *my_malloc(size_t size, const char *func, unsigned line) {
  if (size == 0) {
    return NULL;
  }
  void *ptr = malloc(size);
  if (ptr == NULL) {
    fprintf(stderr, "Failed to allocate %zu bytes at \"%s()\", line %u\n",
        size, func, line);
    exit(EXIT_FAILURE);
  }
  return ptr;
}

user_code.c

#include "my_malloc.h"

...
  // No need to explicitly pass the function name, line number
  arg = MY_MALLOC(sizeof *arg * n);
  // Error checking not needed.

  ...
  free(arg);

I'd even consider a matching MY_FREE where freeing passes in the pointer and the expected size. Then those 2 routines could keep allocation statistics, free size validation, ...

CodePudding user response：

If you have many objects, you cam allocate memory for all immediately:

void *pvc;
pvc = malloc(how_many_for_all);

and then consecutively declare pointers to objects like this:

Obj1 *p1;
Obj2 *p2;
Obj3 *p3;
p1 = (Obj1)pvc;
p2 = (Obj2)(pvc   sizeof(Obj1));
p3 = (Obj3)(pvc   sizeof(Obj1)   sizeof(Obj2));

This is pseudocode sooner. Compiler will be make warnings, but this works.