What is the best way to return an error from a function when I'm already returning a value?

Question

I wrote a function in C that converts a string to an integer and returns the integer. When I call the function I also want it to let me know if the string is not a valid number. In the past I returned -1 when this error occurred, because I didn't need to convert strings to negative numbers. But now I want it to convert strings to negative numbers, so what is the best way to report the error?

In case I wasn't clear about this: I don't want this function to report the error to the user, I want it to report the error to the code that called the function. ("Report" might be the wrong word to use...)

Here's the code:

s32 intval(const char *string) {
    bool negative = false;
    u32 current_char = 0;

    if (string[0] == '-') {
     negative = true;
     current_char = 1;
    }

    s32 num = 0;
    while (string[current_char]) {
     if (string[current_char] < '0' || string[current_char] > '9') {
      // Return an error here.. but how?
     }

     num *= 10;
     num += string[current_char] - '0';
     current_char++;
    }

    if (negative) {
     num = -num;
    }

    return num;
}

Answer 1

a common way is to pass a pointer to a success flag like this:

int my_function(int *ok) {
    /* whatever */
    if(ok != NULL) {
        *ok = success;
    }
    return ret_val;
}

call it like this:

int ok;
int ret = my_function(&ok);
if(ok) {
    /* use ret safely here */
}

EDIT: example implementation here:

s32 intval(const char *string, int *ok) {
    bool negative = false;
    u32 current_char = 0;

    if (string[0] == '-') {
        negative = true;
        current_char = 1;
    }

    s32 num = 0;
    while (string[current_char]) {
        if (string[current_char] < '0' || string[current_char] > '9') {
                // Return an error here.. but how?
                if(ok) { *ok = 0; }
        }

        num *= 10;
        num += string[current_char] - '0';
        current_char++;
    }

    if (negative) {
        num = -num;
    }
    if(ok) { *ok = 1; }
    return num;
}

int ok;
s32 val = intval("123a", &ok);
if(ok) {
    printf("conversion successful\n");
}

Answer 2

Well, the way that .NET handles this in Int32.TryParse is to return the success/failure, and pass the parsed value back with a pass-by-reference parameter. The same could be applied in C:

int intval(const char *string, s32 *parsed)
{
    *parsed = 0; // So that if we return an error, the value is well-defined

    // Normal code, returning error codes if necessary
    // ...

    *parsed = num;
    return SUCCESS; // Or whatever
}

Answer 3

The os-style global errno variable is also popular. Use errno.h.

If errno is non-zero, something went wrong.

Here's a man page reference for errno.

Answer 4

There are several ways. All have their pluses and minuses:

• have the function return an error code and pass in a pointer to a location to return the result. The nice thing about this there's no overloading of the result. The bad thing is that you can't use the real result of the function directly in an expression.

  Evan Teran suggested a variation of this that has the caller pass a pointer to a result variable (which can be optionally NULL if the caller doesn't care) and returns the actual value from the function. This has the advantage of allowing the function to be used directly in expressions when the caller is OK with a default value in an error result or knows that the function cannot fail.

• use a special 'sentinel' return value to indicate an error, such as a negative number (if normal return values cannot be negative) or INT_MAX or INT_MIN if good values cannot be that extreme. Sometimes to get more detailed error information a call to another function (such as GetLastError()) or a global variable needs to be consulted (such as errno). This doesn't work well when your return value has no invalid values, and is considered bad form in general by many people.

  An example function that uses this technique is getc(), which returns EOF if end of file is reached or an error is encountered.

• have the function never return an error indication directly, but require the caller to query another function or global. This is similar to how VB's "On Error Goto Next" mode works - and it's pretty much universally considered a bad way to go.

• yet another way to go is to have a 'default' value. For example, the atoi() function, which has pretty much the same functionality that your intval() function, will return 0 when it is unable to convert any characters (it's different from your function in that it consumes characters to convert until it reaches the end of string or a character that is not a digit).

  The obvious drawback here is that it can be tricky to tell if an actual value has been converted or if junk has been passed to atoi().

  I'm not a huge fan of this way to handle errors.

I'll update as other options cross my mind...

Answer 5

You can either return an instance of a class where a property would be the value interested in, another property would be a status flag of some sort. Or, pass in an instance of the result class..

Pseudo code
  MyErrStatEnum = (myUndefined, myOK, myNegativeVal, myWhatever)

ResultClass
  Value:Integer;
  ErrorStatus:MyErrStatEnum

Example 1:

result := yourMethod(inputString)

if Result.ErrorStatus = myOK then 
   use Result.Value
else
  do something with Result.ErrorStatus

free result

Example 2

create result
yourMethod(inputString, result)

if Result.ErrorStatus = myOK then 
   use Result.Value
else
  do something with Result.ErrorStatus

free result

The benefit of this approach is you can expand the info coming back at any time by adding additional properties to the Result class.

To expand this concept further, it also applies to method calls with multiple input parameters. For example, instead of CallYourMethod(val1, val2, val3, bool1, bool2, string1) instead, have a class with properties matching val1,val2,val3,bool1,bool2,string1 and use that as a single input parameter. It cleans up the method calls and makes the code more easily modified in the future. I'm sure you've seen that method calls with more than a few parameters is much more difficult to use/debug. (7 is the absolute most I would say.)

Answer 6

In general I prefer the way Jon Skeet proposed, ie. returning a bool (int or uint) about success and storing the result in a passed address. But your function is very similar to strtol, so I think it is a good idea to use the same (or similar) API for your function. If you give it a similar name like my_strtos32, this makes it easy to understand what the function does without any reading of the documentation.

EDIT: Since your function is explicitly 10-based, my_strtos32_base10 is a better name. As long as your function is not a bottle-neck you can then, skip your implementation. And simply wrap around strtol:

s32
my_strtos32_base10(const char *nptr, char **endptr)
{
    long ret;
    ret = strtol(nptr, endptr, 10);
    return ret;
}

If you later realize it as an bottleneck you can still optimize it for your needs.

Answer 7

Take a look at how the standard library deals with this problem:

long  strtol(const  char  *str,  char **restrict endptr, int base);

Here, after the call the endptr points at the first character that could not be parsed. If endptr == str, then no characters were converted, and this is a problem.