Error Handling with the Single Input Single Output Pattern

From RidgeRun Developer Connection
Jump to: navigation, search

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.

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.

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.

 1 static GstFlowReturn
 2 gst_example_extract_raw_buffer (GstNvMediaDemux * demux,
 3     GstBuffer * buffer, GstBuffer ** raw_buffer)
 4 {
 5   gint nvstatus = NVMEDIA_STATUS_OK;
 6   GstFlowReturn ret = GST_FLOW_OK;
 7   guint bytes_per_pixel = 2;
 8   NvMediaImageSurfaceMap surface_map = { 0 };
 9   NvMediaImage *image = NULL;
10   GstNvMediaMeta *meta = NULL;
11   guint8 *data = NULL;
12 
13   g_return_val_if_fail (demux, GST_FLOW_ERROR);
14   g_return_val_if_fail (buffer, GST_FLOW_ERROR);
15   g_return_val_if_fail (raw_buffer, GST_FLOW_ERROR);
16 
17   *raw_buffer = NULL;
18 
19   meta = gst_buffer_get_nv_media_meta (buffer);
20 
21   image = meta->image_raw;
22   nvstatus = NvMediaImageLock (image, NVMEDIA_IMAGE_ACCESS_READ,
23       &surface_map);
24 
25   image_size = gst_example_compute_image_size (&surface_map); 
26   data = (guint8 *) g_malloc (image_size * sizeof (guint8));
27 
28   nvstatus = NvMediaImageGetBits (image, data, image_size);
29   
30   *raw_buffer = gst_buffer_new_wrapped (buff, image_size);
31 
32   gst_example_analyze (*raw_buffer);
33 
34   NvMediaImageUnlock (meta->image_raw);
35   
36   return ret;
37 }

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:

 1 static GstFlowReturn
 2 gst_example_extract_raw_buffer (GstNvMediaDemux * demux,
 3     GstBuffer * buffer, GstBuffer ** raw_buffer)
 4 {
 5   /* Declarations and pointer guards */
 6 
 7   meta = gst_buffer_get_nv_media_meta (buffer);
 8   if (NULL == meta) {
 9     GST_ERROR_OBJECT (demux, "No NvMedia meta found, is the and NvMedia buffer?");
10     return GST_FLOW_ERROR;
11   }
12 
13   image = meta->image_raw;
14   nvstatus = NvMediaImageLock (image, NVMEDIA_IMAGE_ACCESS_READ,
15       &surface_map);
16   if (NVMEDIA_STATUS_OK != nvstatus) {
17     GST_ERROR_OBJECT (demux, "Unable to lock NvMedia image: %d", nvstatus);
18     return GST_FLOW_ERROR;
19   }
20 
21   image_size = gst_example_compute_image_size (&surface_map); 
22   data = (guint8 *) g_malloc (image_size * sizeof (guint8));
23   if (NULL == data) {
24     GST_ERROR_OBJECT (demux, "Unable to alloc buffer, system ran out of memory");
25     return GST_FLOW_ERROR;
26   }
27 
28   nvstatus = NvMediaImageGetBits (image, data, image_size);
29   if (NVMEDIA_STATUS_OK != nvstatus) {
30     GST_ERROR_OBJECT (demux, "Unable to lock NvMedia image: %d", nvstatus);
31     return GST_FLOW_ERROR;
32   }
33   
34   *raw_buffer = gst_buffer_new_wrapped (buff, image_size);
35   if (NULL == *raw_buffer) {
36     GST_ERROR_OBJECT (demux, "Unable to wrap data, system ran out of memory");
37     return GST_FLOW_ERROR;
38   }
39 
40   if (FALSE == gst_example_analyze (*raw_buffer)) {
41     GST_ERROR_OBJECT (demux, "Some business logic error");
42     return GST_FLOW_ERROR;
43   }
44   
45   NvMediaImageUnlock (meta->image_raw);
46   
47   return ret;
48 }

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.

 1 static GstFlowReturn
 2 gst_example_extract_raw_buffer (GstNvMediaDemux * demux,
 3     GstBuffer * buffer, GstBuffer ** raw_buffer)
 4 {
 5   /* Declarations and pointer guards */
 6 
 7   meta = gst_buffer_get_nv_media_meta (buffer);
 8   if (NULL == meta) {
 9     GST_ERROR_OBJECT (demux, "No NvMedia meta found, is the and NvMedia buffer?");
10     return GST_FLOW_ERROR;
11   }
12 
13   image = meta->image_raw;
14   nvstatus = NvMediaImageLock (image, NVMEDIA_IMAGE_ACCESS_READ,
15       &surface_map);
16   if (NVMEDIA_STATUS_OK != nvstatus) {
17     GST_ERROR_OBJECT (demux, "Unable to lock NvMedia image: %d", nvstatus);
18     return GST_FLOW_ERROR;
19   }
20 
21   image_size = gst_example_compute_image_size (&surface_map); 
22   data = (guint8 *) g_malloc (image_size * sizeof (guint8));
23   if (NULL == data) {
24     GST_ERROR_OBJECT (demux, "Unable to alloc buffer, system ran out of memory");
25     NvMediaImageUnlock (meta->image_raw); /* <---- Freeing a resource  */
26     return GST_FLOW_ERROR;
27   }
28 
29   nvstatus = NvMediaImageGetBits (image, data, image_size);
30   if (NVMEDIA_STATUS_OK != nvstatus) {
31     GST_ERROR_OBJECT (demux, "Unable to lock NvMedia image: %d", nvstatus);
32     NvMediaImageUnlock (meta->image_raw); /* <---- Freeing a resource  */
33     g_free (data); /* <--------------------------- Freeing a resource  */
34     return GST_FLOW_ERROR;
35   }
36   
37   *raw_buffer = gst_buffer_new_wrapped (buff, image_size);
38   if (NULL == *raw_buffer) {
39     GST_ERROR_OBJECT (demux, "Unable to wrap data, system ran out of memory");
40     NvMediaImageUnlock (meta->image_raw); /* <---- Freeing a resource  */
41     g_free (data); /* <--------------------------- Freeing a resource  */
42     return GST_FLOW_ERROR;
43   }
44 
45   if (FALSE == gst_example_analyze (*raw_buffer)) {
46     GST_ERROR_OBJECT (demux, "Some business logic error");
47     NvMediaImageUnlock (meta->image_raw); /* <---- Freeing a resource  */
48     gst_buffer_unref (*raw_buf); /* <------------- Freeing a resource  */
49     return GST_FLOW_ERROR;
50   }
51   
52   NvMediaImageUnlock (meta->image_raw);
53   
54   return ret;
55 }

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:

Analyzing the resource acquisition


RidgeRun Resources

Quick Start Client Engagement Process RidgeRun Blog Homepage
Technical and Sales Support RidgeRun Online Store RidgeRun Videos Contact Us

OOjs UI icon message-progressive.svg Contact Us

Visit our Main Website for the RidgeRun Products and Online Store. RidgeRun Engineering informations are available in RidgeRun Professional Services, RidgeRun Subscription Model and Client Engagement Process wiki pages. Please email to support@ridgerun.com for technical questions and contactus@ridgerun.com for other queries. Contact details for sponsoring the RidgeRun GStreamer projects are available in Sponsor Projects page. Ridgerun-logo.svg
RR Contact Us.png
  1. No resources are freed here, leading to leaks and potential deadlocks!