Difference between revisions of "Error Handling with the Single Input Single Output Pattern"

From RidgeRun Developer Connection
Jump to: navigation, search
Line 13: Line 13:
 
The single output point is way more common but gladly less harmful. The simplest example being having multiple returns in a single function. This is very typical during error handling or short-circuiting based on certain conditions.
 
The single output point is way more common but gladly less harmful. The simplest example being having multiple returns in a single function. This is very typical during error handling or short-circuiting based on certain conditions.
  
[[File:Siso_intro.png.png|thumb|Examples of different amount of entry and output points]]
+
[[File:Siso intro|thumb|center|Examples of different amounts of entry and output points.]]
  
 
=== GoTo Disclaimers ===
 
=== GoTo Disclaimers ===

Revision as of 12:02, 28 August 2020

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 de 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 being having multiple returns in a single function. This is very typical during error handling or short-circuiting based on certain conditions.

File:Siso intro
Examples of different amounts of entry and output points.

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.
  • No, you shouldn't be applying this to C++. Instead I recommend the use of smart pointers or any other RAII.

The Problem

Consider the following hypothetical example.

static gboolean
example_extract_raw_buffer (GstNvMediaDemux * demux,
    GstBuffer * buffer, GstBuffer ** raw_buffer)
gint res = NVMEDIA_STATUS_OK;
  GstFlowReturn ret = GST_FLOW_OK;
  guint bytes_per_pixel;
  NvMediaImageSurfaceMap surface_map;
  gint image_size;
  guint pbuff_pitches;
  guint8 *pbuff[1];
  guint8 *buff;
  GstNvMediaMeta *inmeta = NULL;

  NVM_SURF_FMT_DEFINE_ATTR (src_attr);
  *raw_buffer = NULL;

  inmeta = (GstNvMediaMeta *) gst_buffer_get_meta (buffer,
      GST_NV_MEDIA_META_API_TYPE);
  if (!inmeta) {
    GST_ERROR_OBJECT (demux,
        "NvMedia meta not available, is this an NvMedia buffer?");
    return GST_FLOW_ERROR;
  }

  res =
      NvMediaSurfaceFormatGetAttrs (inmeta->image_raw->type, src_attr,
      NVM_SURF_FMT_ATTR_MAX);

  if (res != NVMEDIA_STATUS_OK) {
    GST_ERROR_OBJECT (demux, "NvMediaSurfaceFormatGetAttrs failed");
    ret = GST_FLOW_ERROR;
    goto buff_err;
  }

  switch (src_attr[NVM_SURF_ATTR_BITS_PER_COMPONENT].value) {
    case NVM_SURF_ATTR_BITS_PER_COMPONENT_8:
      bytes_per_pixel = 1;
      break;
    case NVM_SURF_ATTR_BITS_PER_COMPONENT_10:
    case NVM_SURF_ATTR_BITS_PER_COMPONENT_12:
    case NVM_SURF_ATTR_BITS_PER_COMPONENT_14:
    case NVM_SURF_ATTR_BITS_PER_COMPONENT_16:
      bytes_per_pixel = 2;
      break;
    case NVM_SURF_ATTR_BITS_PER_COMPONENT_20:
    case NVM_SURF_ATTR_BITS_PER_COMPONENT_32:
      bytes_per_pixel = 4;
      break;
    default:
      GST_ERROR_OBJECT (demux, "Invalid surface type");
      ret = GST_FLOW_ERROR;
      goto buff_err;
  }

  res =
      NvMediaImageLock (inmeta->image_raw, NVMEDIA_IMAGE_ACCESS_READ,
      &surface_map);

  if (res != NVMEDIA_STATUS_OK) {
    GST_ERROR_OBJECT (demux, "NvMediaImageLock failed");
    ret = GST_FLOW_ERROR;
    goto buff_err;
  }

  image_size = surface_map.width * surface_map.height * bytes_per_pixel;
  image_size += inmeta->image_raw->embeddedDataTopSize;
  image_size += inmeta->image_raw->embeddedDataBottomSize;

  buff = (guint8 *) g_malloc (image_size);

  pbuff_pitches = surface_map.width * bytes_per_pixel;
  pbuff[0] = buff;

  if (NULL == buff) {
    GST_ERROR_OBJECT (demux, "System ran out of memory");
    ret = GST_FLOW_ERROR;
    goto buff_err;
  }

  res =
      NvMediaImageGetBits (inmeta->image_raw, NULL, (void **) pbuff,
      &pbuff_pitches);

  if (res != NVMEDIA_STATUS_OK) {
    GST_ERROR_OBJECT (demux, "NvMediaImageGetBits failed");
    g_free (buff);
    ret = GST_FLOW_ERROR;
    goto buff_err;
  }

  *raw_buffer = gst_buffer_new_wrapped (buff, image_size);

buff_err:
  NvMediaImageUnlock (inmeta->image_raw);
  return ret;
}