Best/Cleanest way to structure error checking on dynamically allocated variables

Question

I am working on a Python C project that involves a couple of dynamically allocated structures like queues and threading structures. I am mainly looking to improve the readability. It looks something like this:

#include <Python.h>
#include <stdlib.h>

typedef struct Queue{
   void *data;
} Queue;
typedef struct Control{
   void *data;
} Control;
typedef struct Logging{
   void *data;
} Logging;
typedef struct Args{
   Queue *q;
   Control *c;
   Logging *l;
   void *data;
} Args;

void cleanupQueue(Queue *q);
void cleanupControl(Control *c);
void cleanupLogging(Logging *l);
void cleanupArgs(Args *a);
static PyObject *pycalledfunction(){
  Queue *queue = (Queue *) malloc(sizeof(Queue));
  if(!queue){
    PyErr_SetString(type, message);
    return NULL;
  }
  // initialize queue and fill
  
  Control *control = (Control *) malloc(sizeof(Control));
  if(!control){
    cleanupQueue(queue);
    PyErr_SetString(type, message);
    return NULL;
  }
  // initialize control
  
  Logging *logging = (Logging *) malloc(sizeof(Logging));
  if(!logging){
    cleanupControl(control);
    cleanupQueue(queue);
    PyErr_SetString(type, message);
    return NULL;
  }
  // initialize logging
  
  Args *args = (Args *) malloc(sizeof(Args));
  if(!logging){
    cleanupLogging(logging);
    cleanupControl(control);
    cleanupQueue(queue);
    PyErr_SetString(type, message);
    return NULL;
  }
  // initialize and fill args with the other structs
  
  // Note: I guess I could check if they all exist before doing work with them here but I was unsure if that is a good practice since I am loading them into the args and am keen on getting the correct error message.
  
  // bunch of work here
  
  cleanupQueue(queue);
  cleanupControl(control);
  cleanupLogging(logging);
  cleanupArgs(args);
  return obj;
}

Now this code works just fine as long as it cleans up the allocated memory properly; although it lowers the flow of the code and its overall readability by a small margin, I am still looking to improve my structuring practices by any margin. I left a note in there marking that I could check each of the structures before doing any work(having all of the checking in the same place), but I feel as if this would result in the same flow.

Is there any way to do this in a way that ensures that I return the correct error while improving the structure of my code?

EDIT: Decided that goto statements are sufficient enough to clean the code enough(while maintaining good practice). If there are any other methods that you believe clean the code further, send me a message to reopen.

EDIT: Note of semi-duplicate to this question

Answer 1

Based on the answers from @Harith and @John Bollinger (as well as the comment from @Ted Lyngmo), combining the two approaches to structure [nested-ifs, goto] may output the cleanest structure. Using the example:

static PyObject *pycalledfunction()
{
  PyObject *error_typ = NULL;
  PyObject *rval = NULL;
  char *error_msg = NULL;
  Queue *queue = malloc(sizeof *queue);
  if (!queue){
      error_typ = PyExc_RuntimeError; // insert proper error type here
      error_msg = "failed to allocate Queue";
      goto fail_queue;
  }
  
  Control *control = malloc(sizeof *control);
  if (!control){
      error_typ = PyExc_RuntimeError; // insert proper error type here
      error_msg = "failed to allocate Control";
      goto fail_control;
  }
  
  Logging *logging = malloc(sizeof *logging);
  if (!logging){
      error_typ = PyExc_RuntimeError; // insert proper error type here
      error_msg = "failed to allocate Logging";
      goto fail_logging;
  }
  
  Args *args = malloc(sizeof *args);
  if (!args){
      error_typ = PyExc_RuntimeError; // insert proper error type here
      error_msg = "failed to allocate Args";
      goto fail_args;
  }
fail_args:
   cleanupArgs(args);
fail_logging:
   cleanupLogging(logging);
fail_control:
   cleanupControl(control);
fail_queue:
   cleanupQueue(queue);
  
  if(error_typ || error_msg){
     PyErr_SetString(type, message);
     return NULL;
  }

  return rval;
}

This approach allows the reduction of wet code that the nested-ifs and the simplicity of the goto statements. To reduce the lines of code in the checking, another structure could be implemented(although almost unnecessary):

typedef struct MyPyError {
   PyObject type;
   char *msg;
} MyPyError;

static PyObject *pycalledfunction()
{
   MyPyError error;

   // ...
   if(!queue){
      error = (MyPyError) {PyExc_RuntimeError, "failed to allocate queue"};
      goto fail_queue;
   }
   // ...
   if(error){
      PyErr_SetString(error.type, error.msg);
      return NULL;
   }
   return rval;
}
   

Answer 2

As I understand the question, it is about how on passing each point of possible allocation failure, you accumulate one more cleanup function to call. You end up with multiple, partially-duplicate lists of cleanup calls.

One way to approach this issue would be by nesting, so that the cleanup logic flows naturally:

  PyObject *rval = NULL;

  Queue *queue = malloc(sizeof(*queue));
  if(!queue){
    PyErr_SetString(type, message);
  } else {
    // initialize queue and fill
  
    Control *control = malloc(sizeof(*control));
    if(!control){
      PyErr_SetString(type2, message2);
    } else {
      // initialize control
  
      Logging *logging = malloc(sizeof(*logging));
      if(!logging) {
        PyErr_SetString(type3, message3);
      } else {
        // initialize logging
  
        Args *args = malloc(sizeof(*args));
        if(!args) {
          PyErr_SetString(type4, message4);
        } else {
          // initialize and fill args with the other structs
  
          // ... bunch of work here ...

          rval = obj;

          cleanupArgs(args);
        }
        cleanupLogging(logging);
      }
      cleanupControl(control);
    }
    cleanupQueue(queue);
  }
  return rval;

You could also do similar by putting each of those nested blocks into its own function, each one calling the next until you ultimately get to the one doing the actual work. That might be more satisfactory to some programmers' sensibilities.

And you could do similar with goto, as your other answer suggests. I consider that a reasonable and legitimate use of goto, and I have personally written similar error-handling code using goto. However, as a personal preference, I do prefer to reserve goto for cases that cannot readily be handled another way. As you depict it, this is not such a case.

Answer 3

You can pull out good ol' goto (this is one valid use-case of goto):

static PyObject *pycalledfunction()
{
  Queue *queue = malloc(sizeof *queue);

  if (!queue){
      goto fail_queue;
  }
  
  Control *control = malloc(sizeof *control);

  if (!control){
      goto fail_control;
  }
  
  Logging *logging = malloc(sizeof *logging);

  if (!logging){
      goto fail_logging;
  }
  
  Args *args = malloc(sizeof *args);

  if (!args){
      goto fail_args;
  }

  cleanupQueue(queue);
  cleanupControl(control);
  cleanupLogging(logging);
  cleanupArgs(args);
  return obj;
  
fail_args:
  cleanupLogging(logging);
fail_logging:
    cleanupControl(control);
fail_control:
    cleanupQueue(queue);
fail_queue:
    PyErr_SetString(type, message);
    return NULL;
}

My labels may not be creative enough, rename them however you may. Or use the nested approach that @Jonathan Leffer shows here: https://stackoverflow.com/questions/788903/valid-use-of-goto-for-error-management-in-c.