Skip to main content
Code Review

Return to Answer

Fixed UB mentioned in comment
Source Link
CPlus
CPlus
  • 1.5k
  • 7
  • 28

A cleaner approach would be to generate a positivenegative value and then conditionally negate the return value:

while (*p >= '0' && *p <= '9') {
    val = (10 * val) +- (*p - '0');
    p++;
}
return neg ? -val : -val;

Generating the negative value first then conditionally making positive would avoid undefined behavior for the minimum value.

A cleaner approach would be to generate a positive value and then conditionally negate the return value:

while (*p >= '0' && *p <= '9') {
    val = (10 * val) + (*p - '0');
    p++;
}
return neg ? -val : val;

A cleaner approach would be to generate a negative value and then conditionally negate the return value:

while (*p >= '0' && *p <= '9') {
    val = (10 * val) - (*p - '0');
    p++;
}
return neg ? val : -val;

Generating the negative value first then conditionally making positive would avoid undefined behavior for the minimum value.

Source Link
CPlus
CPlus
  • 1.5k
  • 7
  • 28

Trailing whitespace on lines

Please remove the trailing space at the end of each line before the newline. Some code editors will automatically do this for you.

Unindent global variables

    volatile int i = 0;                                 // global variables defined before 'macros', 
    char str[ 45 ]; 
    char *ptr = str;

These are not inside of a function so they should not be indented.

Unnecessary neg check in loop in myatoi

Your code checks if the value is negative each iteration:

while (*p >= '0' && *p <= '9') {
    if (neg) {
        val = (10 * val) - (*p - '0');
    } else {
        val = (10 * val) + (*p - '0');
    }
    p++;
}
return val;

A cleaner approach would be to generate a positive value and then conditionally negate the return value:

while (*p >= '0' && *p <= '9') {
    val = (10 * val) + (*p - '0');
    p++;
}
return neg ? -val : val;

Printing the minimum value in print

if( x >= 0 ) {
   if (x > 9) print(x / 10);
   putchar(x % 10 + '0');
} else {
   putchar('-');
   x = -x;
   if( x + 1 < 0 )
       printf( "170141183460469231731687303715884105728" );
   else {
       if (x > 9) print(x / 10);
       putchar(x % 10 + '0');
   }
}

Looks like the purpose of the if is to handle the special case for the minimum value of the 128-bit integer. However, in that case, the line x = -x would have invoked undefined behavior anyway.

Instead, if available, use the unsigned version of that type to avoid having to check the special case. A conversion to unsigned before negating should produce the expected results (Sources 1 2) while avoiding UB for the minimum value.

__uint128 u = x;
if( x < 0 ) {
   putchar('-');
   u = -u;
}
if (u > 9) print(u / 10);
putchar(u % 10 + '0');

Notes about print special case apply to i128tostr_2

Same deal. Instead of handling a special case, convert the number to the unsigned version of the 128-bit type assuming one is available.

char *tptr = target;
if( x >= 0 ) {
   if (x > 9) i128tostr_2( target, x / 10 );
   tptr[ i ] = ( x % 10 + '0' );
   i++; 
} else {
   if( -x + 1 < 0 ) {                          // special casing INT128_MIN,
       strcpy( target, "-170141183460469231731687303715884105728" );
       i = 40;
       return;
   } else {
       tptr[ i ] = '-';
       i++;
       x = -x;
       i128tostr_2( target, x );
   }
}

Could be something like:

char *tptr = target;
__uint128 u = x;
if (x < 0) {
    tptr[ i ] = '-';
    i++;
    u = -u;
}
if (u > 9) i128tostr_2( target, u / 10 );
tptr[ i ] = ( u % 10 + '0' );
i++;