HiveBrain v1.2.0
Get Started
← Back to all entries
debugcMinor

Error handling for function calls to parse a GIF file

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
callshandlingerrorfilegiffunctionparsefor

Problem

I'm debating which version would be more suitable in parameters of code length, readability and maintenance. This is a portion of a set of functions which together parse a Gif file.

Version 1:

if ((errno = GifLoadHeaders(g, &bufTemp)) != GIF_SUCCESS) {
    GifDispose(g);
    *gPtr = NULL;

    return errno;
}

if ((errno = GifLoadGCT(g, &bufTemp)) != GIF_SUCCESS) {
    GifDispose(g);
    *gPtr = NULL;

    return errno;
}

if ((errno = GifLoadImages(g, &bufTemp)) != GIF_SUCCESS) {
    GifDispose(g);
    *gPtr = NULL;

    return errno;
}


Version 2:

if ((errno = GifLoadHeaders(g, &bufTemp)) != GIF_SUCCESS
    || (errno = GifLoadGCT(g, &bufTemp)) != GIF_SUCCESS
    || (errno = GifLoadImages(g, &bufTemp)) != GIF_SUCCESS) {

    GifDispose(g);
    *gPtr = NULL;

    return errno;
}


I generally prefer the latter version. Maybe there's a more optimal third solution to my issue. Any help?

EDIT:
I'm sorry I did not include the entire routine. The code I've included is used in the function GifParse which initializes a given pointer to a Gif structure, which is named gPtr.

Solution

Without knowing the context, I'd write it as:

if( (errno = GifLoad( g, &bufTemp )) == GIF_SUCCESS ) {
  // Process the correctly loaded image as necessary.
  GifProcess( g );
}

// Release memory once processing is complete.
GifDispose( g );
*gPtr = NULL;

// Single exit point to the function.
return errno;


Then write a wrapper for GifLoad that calls the three other functions.

This might not be a suitable solution if, for example, the processing is part of an interactive program wherein the processing is indeterminate. If processing cannot be coded at this point in the source, then consider:

if( (errno = GifLoad( g, &bufTemp )) != GIF_SUCCESS ) {
  // Perform whatever is necessary to inform the user the GIF cannot be loaded.
  GifInvalid( g );

  // Release memory.
  GifDispose( g );
  *gPtr = NULL;
}

return errno;


In both cases consider abstracting the three function calls into a single function, which cleans up the code and expresses the intent of those three function calls more concisely.

Code Snippets

if( (errno = GifLoad( g, &bufTemp )) == GIF_SUCCESS ) {
  // Process the correctly loaded image as necessary.
  GifProcess( g );
}

// Release memory once processing is complete.
GifDispose( g );
*gPtr = NULL;

// Single exit point to the function.
return errno;
if( (errno = GifLoad( g, &bufTemp )) != GIF_SUCCESS ) {
  // Perform whatever is necessary to inform the user the GIF cannot be loaded.
  GifInvalid( g );

  // Release memory.
  GifDispose( g );
  *gPtr = NULL;
}

return errno;

Context

StackExchange Code Review Q#36083, answer score: 4

Revisions (0)

No revisions yet.