Error Handling with the Single Input Single Output Pattern

Introduction

The single-input single-output is a simple pattern for procedural C code that eases:

Graceful error handling
Avoid code duplication
Improve readability.

Despite this, it is quite controversial among the developer's community due to the use of the infamous goto operator. The idea in single-input single-output is simple: Every function should have a single entry point and a single output point.

The single entry point is natural for most of us, and probably (hopefully) de 100% of the cases. It is breakable though. In some old non-standard C compilers you could jump (using a goto) to a line in another function. Other more day-to-day examples include the use of setjump or longjump.

The single output point is way more common but gladly less harmful. The simplest example is having multiple returns in a single function. This is very typical during error handling or short-circuiting based on certain conditions.

GoTo Disclaimers

First of all, this is my personal opinion. Having said that:

I do not encourage the arbitrary use of goto jumps.
The single-input single-output is one of the few cases in which I believe is acceptable, even beneficial.
Yes, you can achieve the same with nested if/else structures. No, it won't be as readable and clean.

Applying the Pattern

Consider the following excerpt from a code I just recently reviewed. I've simplified it for readability. This code has, deliberately, no error handling. This is what we are trying to solve.

static GstFlowReturn
gst_example_extract_raw_buffer (GstNvMediaDemux * demux,
    GstBuffer * buffer, GstBuffer ** raw_buffer)
{
  gint nvstatus = NVMEDIA_STATUS_OK;
  GstFlowReturn ret = GST_FLOW_OK;
  guint bytes_per_pixel = 2;
  NvMediaImageSurfaceMap surface_map = { 0 };
  NvMediaImage *image = NULL;
  GstNvMediaMeta *meta = NULL;
  guint8 *data = NULL;

  g_return_val_if_fail (demux, GST_FLOW_ERROR);
  g_return_val_if_fail (buffer, GST_FLOW_ERROR);
  g_return_val_if_fail (raw_buffer, GST_FLOW_ERROR);

  *raw_buffer = NULL;

  meta = gst_buffer_get_nv_media_meta (buffer);

  image = meta->image_raw;
  nvstatus = NvMediaImageLock (image, NVMEDIA_IMAGE_ACCESS_READ,
      &surface_map);

  image_size = gst_example_compute_image_size (&surface_map); 
  data = (guint8 *) g_malloc (image_size * sizeof (guint8));

  nvstatus = NvMediaImageGetBits (image, data, image_size);
  
  *raw_buffer = gst_buffer_new_wrapped (buff, image_size);

  gst_example_analyze (*raw_buffer);

  NvMediaImageUnlock (meta->image_raw);
  
  return ret;
}

Error Handling

This code has to error handling at all. Let's add some, shall we? One first approach can be thought of as the following:

static GstFlowReturn
gst_example_extract_raw_buffer (GstNvMediaDemux * demux,
    GstBuffer * buffer, GstBuffer ** raw_buffer)
{
  /* Declarations and pointer guards */

  meta = gst_buffer_get_nv_media_meta (buffer);
  if (NULL == meta) {
    GST_ERROR_OBJECT (demux, "No NvMedia meta found, is the and NvMedia buffer?");
    return GST_FLOW_ERROR;
  }

  image = meta->image_raw;
  nvstatus = NvMediaImageLock (image, NVMEDIA_IMAGE_ACCESS_READ,
      &surface_map);
  if (NVMEDIA_STATUS_OK != nvstatus) {
    GST_ERROR_OBJECT (demux, "Unable to lock NvMedia image: %d", nvstatus);
    return GST_FLOW_ERROR;
  }

  image_size = gst_example_compute_image_size (&surface_map); 
  data = (guint8 *) g_malloc (image_size * sizeof (guint8));
  if (NULL == data) {
    GST_ERROR_OBJECT (demux, "Unable to alloc buffer, system ran out of memory");
    return GST_FLOW_ERROR;
  }

  nvstatus = NvMediaImageGetBits (image, data, image_size);
  if (NVMEDIA_STATUS_OK != nvstatus) {
    GST_ERROR_OBJECT (demux, "Unable to lock NvMedia image: %d", nvstatus);
    return GST_FLOW_ERROR;
  }
  
  *raw_buffer = gst_buffer_new_wrapped (buff, image_size);
  if (NULL == *raw_buffer) {
    GST_ERROR_OBJECT (demux, "Unable to wrap data, system ran out of memory");
    return GST_FLOW_ERROR;
  }

  if (FALSE == gst_example_analyze (*raw_buffer)) {
    GST_ERROR_OBJECT (demux, "Some business logic error");
    return GST_FLOW_ERROR;
  }
  
  NvMediaImageUnlock (meta->image_raw);
  
  return ret;
}

This is the classical single input multiple output' scenario. Hope you noticed the horrors in this approach :)^[1]

Freeing Resources

Let's deal with the forgotten resources.

static GstFlowReturn
gst_example_extract_raw_buffer (GstNvMediaDemux * demux,
    GstBuffer * buffer, GstBuffer ** raw_buffer)
{
  /* Declarations and pointer guards */

  meta = gst_buffer_get_nv_media_meta (buffer);
  if (NULL == meta) {
    GST_ERROR_OBJECT (demux, "No NvMedia meta found, is the and NvMedia buffer?");
    return GST_FLOW_ERROR;
  }

  image = meta->image_raw;
  nvstatus = NvMediaImageLock (image, NVMEDIA_IMAGE_ACCESS_READ,
      &surface_map);
  if (NVMEDIA_STATUS_OK != nvstatus) {
    GST_ERROR_OBJECT (demux, "Unable to lock NvMedia image: %d", nvstatus);
    return GST_FLOW_ERROR;
  }

  image_size = gst_example_compute_image_size (&surface_map); 
  data = (guint8 *) g_malloc (image_size * sizeof (guint8));
  if (NULL == data) {
    GST_ERROR_OBJECT (demux, "Unable to alloc buffer, system ran out of memory");
    NvMediaImageUnlock (meta->image_raw); /* <---- Freeing a resource  */
    return GST_FLOW_ERROR;
  }

  nvstatus = NvMediaImageGetBits (image, data, image_size);
  if (NVMEDIA_STATUS_OK != nvstatus) {
    GST_ERROR_OBJECT (demux, "Unable to lock NvMedia image: %d", nvstatus);
    NvMediaImageUnlock (meta->image_raw); /* <---- Freeing a resource  */
    g_free (data); /* <--------------------------- Freeing a resource  */
    return GST_FLOW_ERROR;
  }
  
  *raw_buffer = gst_buffer_new_wrapped (buff, image_size);
  if (NULL == *raw_buffer) {
    GST_ERROR_OBJECT (demux, "Unable to wrap data, system ran out of memory");
    NvMediaImageUnlock (meta->image_raw); /* <---- Freeing a resource  */
    g_free (data); /* <--------------------------- Freeing a resource  */
    return GST_FLOW_ERROR;
  }

  if (FALSE == gst_example_analyze (*raw_buffer)) {
    GST_ERROR_OBJECT (demux, "Some business logic error");
    NvMediaImageUnlock (meta->image_raw); /* <---- Freeing a resource  */
    gst_buffer_unref (*raw_buf); /* <------------- Freeing a resource  */
    return GST_FLOW_ERROR;
  }
  
  NvMediaImageUnlock (meta->image_raw);
  
  return ret;
}

Do you see how the error handling starts to pile up with duplicate code?

Refactoring to SISO

First, let's analyze the resources acquisition and their order:

Contact Us

For direct inquiries, please refer to the contact information available on our Contact page. Alternatively, you may complete and submit the form provided at the same link. We will respond to your request at our earliest opportunity.

Links to RidgeRun Resources and RidgeRun Artificial Intelligence Solutions can be found in the footer below.

↑ No resources are freed here, leading to leaks and potential deadlocks!

❯

[1] No resources are freed here, leading to leaks and potential deadlocks!

[1]