3
\$\begingroup\$

I'm currently learning C with The C Programming Language by K&R and this is my current exercise:

Rewrite appropriate programs from earlier chapters and exercises with pointers instead of array indexing. Good possibilities include getline, atoi, itoa, reverse, strindex and getop.

This is my atoi solution with pointers:

#include <ctype.h>

int atoi(const char *nptr)
{
    while(isspace(*nptr++)) // skip white space
        ;

    long long val = 0;
    const char *start = nptr;

    if(*nptr == '-' || *nptr == '+')
        nptr++;

    while(*nptr >= '0' && *nptr <= '9')
        val = val*10 + *nptr++ - '0';

    return (*start == '-') ? -val : val;
}

I'm currently a little bit unhappy with the idea, that my code can't do anything against values smaller than INT_MIN and values greater than INT_MAX, but I can't come up with a solution that fits my expectations of good code.

  • I could just return INT_MIN and INT_MAX, but then the caller can't decide between the actual values and an error in form of INT_MIN/INT_MAX.

  • It is also possible to use a second argument - a pointer to an int. The function will write the value into this int and will return 1 if it was successful and 0 if it wasn't.

I want to hear your thoughts on how I should handle this special case.

\$\endgroup\$
6
\$\begingroup\$

You have a bug right on the first statement:

while(isspace(*nptr++)) // skip white space
    ;

I didn't vote to close your post as off-topic for broken code to make a point: it might seem cool to do two things at once in a loop condition (condition + increment), it's not recommended, for two reasons:

  • It's best to have one statement per line
    • A for (;;) is an exception, where it's allowed
  • The loop condition should be about the condition. Leave it at that, move the incrementing logic inside the loop body in a while loop.

The bug is an off-by-one error: when the first digit is found, the condition is false, but the pointer will be still incremented, moving past the first digit. As a result, a string like "123" gets parsed as 23.

The fix is simple, and follows the above recommendations about writing loops:

// skip white space
while (isspace(*nptr)) {
    nptr++;
}

I'm currently a little bit unhappy with the idea, that my code can't do anything against values smaller than INT_MIN and values greater than INT_MAX, but I can't come up with a solution that fits my expectations of good code.

Hm, the method is declared to return an int... So it seems you don't like that int is limited within INT_MIN and INT_MAX by the language. There's just one thing to do: change the return type to something bigger, like a long!


Finally, when comparing against a variable to multiple files in the same conditional expression, often it's more readable to put the conditions in numerical increasing order, for example instead of this:

while(*nptr >= '0' && *nptr <= '9')

Write like this:

while('0' <= *nptr && *nptr <= '9')
\$\endgroup\$
1
  • \$\begingroup\$ Thank you really much for the answer! I really appreciate all the help I can get. Probably I was a little too rash with the while loop and the increase of the pointer. It also makes way more sense to write while('0' <= *nptr && *nptr <= '9') \$\endgroup\$ Sep 18 '15 at 0:45
2
\$\begingroup\$
  1. Code fails to handle INT_MIN correctly in the rare case when int and long long have the same range. But there is no need for long long.

    Using only int, assume INT_MIN == -2147483648, INT_MAX == 2147483647 and input is "-2147483648". val will attempt to take on the value of 2147483648 and overflow before the final negation. int overflow is undefined behavior. An easy solution involves accumulating the negative sum rather than the positive one. That solution also shows overflow prevention/detection without resorting to a wider signed integer type..

  2. Other have pointed our the while(isspace(... issue. There is more. char may be signed and isspace() is only valid over the unsigned char/EOF range. Better to do:

    while(isspace((unsigned char) *nptr)) {
      nptr++;
    }
    
  3. Best not to re-use the name of a C standard function:

     int LSTL_atoi(const char *nptr);
    
  4. Suggest a function signature that returns an int and some measure of success.

     int LSTL_atoi(const char *nptr, bool *success);
    

...thoughts on how I should handle this special case.

String to int could return a value (function return) and code (address of code passed) to indicate various issues.

int my_atoi(const char *s, enum convert_T *code);

Code           Value
0              INT_MIN ... INT_MAX
Overflow       INT_MIN or INT_MAX
No digits      0
Trailing text  0
\$\endgroup\$
1
  • \$\begingroup\$ @LastSecondsToLive Are you seeking additional review of the code? \$\endgroup\$ Oct 30 '15 at 3:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.