Making a function which takes char* exception-proof

Question

I should preface this question by saying that I'm a Java programmer working on my first C++ application, so perhaps there are certain obvious things I don't know.

Some background on the problem:

Our application had data read from a socket stored in a character array, which was a fixed length message header, containing various fixed length fields i.e.:

char[15] data;

The field positions were 0-5,6-7,8-10,11-13 (the last char is a delimiter). The class which wrapped this header offered convenience functions to retrieve the values for each field:

    int ApiHeader::msg_size(void)
    {
        return std::atoi(std::string(data, 6).c_str());
    }

    int ApiHeader::msg_type(void)
    {
        return std::atoi(std::string(data,6,2).c_str());
    }

    int ApiHeader::msg_subtype(void)
    {
        return std::atoi(std::string(data,8,3).c_str());
    }

    int ApiHeader::seqno(void)
    {
        return std::atoi(std::string(data,11,3).c_str());
    }

I was trying to leverage the std::atoi which requires a c-string, but I needed to "slice" the character array into the sub-arrays which contained each separate field, which I did by constructing a string built from the positions in the data array for each field.

The code worked fine, but a user who was doing some code analysis pointed that the msg_subtype() function (and the 2 other similar ones), were actually unintentionally creating an extra string object.

They were invoking this constructor:

basic_string(const basic_string& other, 
              size_type pos, 
              size_type count = std::basic_string::npos, 
              const Allocator& alloc = Allocator() );

So the code was first constructing a new string to turn the char[] to a string&, and then it was constructing another string invoking the string(string&, int, int) constructor.

I redesigned the code to be more efficient:

int atoi(char* string, int start, int length)
{
    double result = 0;
    char* index = string + start*sizeof(char);
    for(int i=length;i>0; i--)
    {
        result += (*index++ - '0') * pow((float) 10, (float) (i-1));
    }
    return (int) result;
}

There are no strings constructed, and it just iterates the part of the data array specified. The problem is that this code requires that there is no way to detect whether the user passed bad values for start and length. (I'm not exactly sure what happens if user passes a length outside of the array. In my unit test, I just saw it got an uninitialized value).

What can be done to make this code more exception proof?

Some ideas I have are:

• If *index is not within valid range ('0' to '9'), break out of the loop.
• Same as above, except throw an Exception (which at least lets user know something went wrong).
• Just document the method to tell user that if values are bad, behavior is "undefined" (I have seen some methods documented this way).

What would be the best C++ way of handling this issue? Or is there a completely different approach which should be used for this function?

Answer 1

First a question: why do you care that an extra string is being created? Is this function used a lot. If not, then I see no need to change.

But beyond that, although I don't know about C++ cleverness, is your replacement function so much more efficient? You are calling pow repeatedly and using floating point to do an integer job. Here is an alternative that would be cheaper. It returns 0 on success, 1 on failure (bad string) and the number desired is output in result. Notice that string is const.

int my_atoi(const char* string, int start, int length, long *result)
{
    char copy[length+1];
    memcpy(copy, string + start, length);
    copy[length] = '\0';
    char *end;
    *result = strtol(copy, &end, 0);
    return (*end != '\0');
}

You might also omit the start parameter and call with

my_atoi(string+start, length, &res);