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 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.

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' scenarios. Hope you noticed the horrors in this approach :)

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