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

atoi implementation in C

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

Problem

I'm looking for feedback on my atoi implementation here in C. Any feedback would great. I made my own atoi for fun and to learn about it.

I'm not trying to replace the standard library version. Reinventing the wheel is fun to me.

#include  /* for LONG_MAX, INT_MAX */
#include 
#include 
#include 

#define IS_ASCII_DIGIT(c) ((c >= 48) && (c <= 57))

long __my_atoi(char* buffer)
{
    long ret = 0;
    bool neg = false;

    if (*buffer == '-') {
        neg = true;
        buffer++; /* advance to next position to pass ascii check */
    }

    while (*buffer) {
        if (IS_ASCII_DIGIT(*buffer)) {
            ret = ret * 10 + (*buffer - '0');
        } else {
            fprintf(stderr, "Fatal Error: unexpected '%c' passed to %s\n", *buffer, __func__);
            exit(EXIT_FAILURE);
        }
        buffer++;
    }

    return neg ? -ret : ret;
}

int main(void)
{
    printf("%ld\n", __my_atoi("-10004"));
    return EXIT_SUCCESS;
}

Solution


  • If implementing your own version of a standard routine such as atoi(), some form of explanation (comment) is needed. For example, is this supposed to be a drop in replacement for atoi()? Are some aspects deliberately not implemented? Do you have space restrictions that prevent you from using the standard? Do you know that certain string combinations will not exist? This type of information is helpful to anyone reading your code so that they can put things into context.



For the remainder of this review, I shall be assuming that it is meant as an exact replacement for atoi().

-
The header file limits.h is included, but I do not see any current reason for it to be there.

-
IS_ASCII_DIGIT() should not be using the magic numbers of 48 and 57. Values such as '0' and '9' are preferred.

  • It is customary to surround the macro arguments with parentheses. This helps to ensure that the argument is evaluated as a whole to cover the case where the argument is a complex expression.



-
Instead of using IS_ASCII_DIGIT(), I would recommend using isdigit(). This would require including the header file ctype.h.

-
atoi() is supposed to return an int. Your routine returns a long. This indicates either a misnamed routine or an incorrect return value.

  • The contents of buffer parameter do not change. It would be better to declare it as a 'const char *'.



-
There is nothing preventing the dereferencing a NULL pointer.

-
There is no accounting for white space at the beginning of the string.

  • There is no accounting for the '+' character at the start of the integer sequence.



-
There is no accounting for values that are too large or too small to represent.

-
The standard atoi() routine does not exit or print any message upon a detected error; neither should yours.

For additional information on atoi(), please refer to the following pages:

  • http://pubs.opengroup.org/onlinepubs/009695399/functions/atoi.html



  • http://pubs.opengroup.org/onlinepubs/009695399/functions/strtol.html



Hope this helps.

Context

StackExchange Code Review Q#84629, answer score: 7

Revisions (0)

No revisions yet.