Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

Better to scale the buffer based of INT_MIN or sizeof(int) and test for OOM. See ansi-c-maximum-number-of-characters-printing-a-decimal-int ansi-c-maximum-number-of-characters-printing-a-decimal-int

Better to scale the buffer based of INT_MIN or sizeof(int) and test for OOM. See ansi-c-maximum-number-of-characters-printing-a-decimal-int

Better to scale the buffer based of INT_MIN or sizeof(int) and test for OOM. See ansi-c-maximum-number-of-characters-printing-a-decimal-int

`FromIntegerToString()`
Source Link
chux
  • 36.2k
  • 2
  • 43
  • 96

Just takingTaking on FromStringToInteger() and FromIntegerToString().

int FromStringToInteger(const char inputString[]) { // Add `const`
 int returnResult = 0;
 size_t counter = 0; // should use `size_t` here
 int overflow = 0; // could use `bool` here
 char sign = inputString[0]; 
 if (sign == '-' || sign == '+') { // Look for + sign too
 counter++;
 }
 while (inputString[counter] != '0円') {
 int digit = inputString[counter++];
 if (digit < CONVERT_ASCII_ZERO || digit > CONVERT_ASCII_NINE) {
 fprintf(stderr, invalidInputError, digit);
 return 0;
 }
 digit -= CONVERT_ASCII_ZERO;
 // Test if overflow is about to occur...
 if ((returnResult <= INT_MIN / 10) && 
 ((returnResult < INT_MIN / 10) || (-digit < INT_MIN % 10))) {
 returnResult = INT_MIN;
 overflow = 1;
 break;
 }
 returnResult = returnResult * 10 - digit; // note use of -
 }
 if (sign != '-') {
 if (returnResult < -INT_MAX) {
 returnResult = INT_MAX;
 overflow = 1;
 } else {
 returnResult = -returnResult;
 }
 }
 if (overflow) {
 // At this point, returnResult is INT_MAX or INT_MIN 
 fprintf(stderr, outOfBoundariesError, returnResult);
 return returnResult; // or drop through
 }
 return returnResult;
}

FromIntegerToString()

With all the macros for portability, using 11 is wrong.

Better to scale the buffer based of INT_MIN or sizeof(int) and test for OOM. See ansi-c-maximum-number-of-characters-printing-a-decimal-int

sizeof(char) is always 1. Little point in using it here. If code might change from char to wchar_t, then better to use malloc(n * sizeof *convertedInputUnsignedInt);

// char *convertedInputUnsignedInt = malloc(11 * sizeof(char));
#define INT_STR_SIZE(type) (10*sizeof(type)*CHAR_BIT/33 + 3)
char *convertedInputUnsignedInt = malloc(INT_STR_SIZE(int));
if (convertedInputUnsignedInt == NULL) {
 Handle_OOM();
}

Just taking on FromStringToInteger()

int FromStringToInteger(const char inputString[]) { // Add `const`
 int returnResult = 0;
 size_t counter = 0; // should use `size_t` here
 int overflow = 0; // could use `bool` here
 char sign = inputString[0]; 
 if (sign == '-' || sign == '+') { // Look for + sign too
 counter++;
 }
 while (inputString[counter] != '0円') {
 int digit = inputString[counter++];
 if (digit < CONVERT_ASCII_ZERO || digit > CONVERT_ASCII_NINE) {
 fprintf(stderr, invalidInputError, digit);
 return 0;
 }
 digit -= CONVERT_ASCII_ZERO;
 // Test if overflow is about to occur...
 if ((returnResult <= INT_MIN / 10) && 
 ((returnResult < INT_MIN / 10) || (-digit < INT_MIN % 10))) {
 returnResult = INT_MIN;
 overflow = 1;
 break;
 }
 returnResult = returnResult * 10 - digit; // note use of -
 }
 if (sign != '-') {
 if (returnResult < -INT_MAX) {
 returnResult = INT_MAX;
 overflow = 1;
 } else {
 returnResult = -returnResult;
 }
 }
 if (overflow) {
 // At this point, returnResult is INT_MAX or INT_MIN 
 fprintf(stderr, outOfBoundariesError, returnResult);
 return returnResult; // or drop through
 }
 return returnResult;
}

Taking on FromStringToInteger() and FromIntegerToString().

int FromStringToInteger(const char inputString[]) { // Add `const`
 int returnResult = 0;
 size_t counter = 0; // should use `size_t` here
 int overflow = 0; // could use `bool` here
 char sign = inputString[0]; 
 if (sign == '-' || sign == '+') { // Look for + sign too
 counter++;
 }
 while (inputString[counter] != '0円') {
 int digit = inputString[counter++];
 if (digit < CONVERT_ASCII_ZERO || digit > CONVERT_ASCII_NINE) {
 fprintf(stderr, invalidInputError, digit);
 return 0;
 }
 digit -= CONVERT_ASCII_ZERO;
 // Test if overflow is about to occur...
 if ((returnResult <= INT_MIN / 10) && 
 ((returnResult < INT_MIN / 10) || (-digit < INT_MIN % 10))) {
 returnResult = INT_MIN;
 overflow = 1;
 break;
 }
 returnResult = returnResult * 10 - digit; // note use of -
 }
 if (sign != '-') {
 if (returnResult < -INT_MAX) {
 returnResult = INT_MAX;
 overflow = 1;
 } else {
 returnResult = -returnResult;
 }
 }
 if (overflow) {
 // At this point, returnResult is INT_MAX or INT_MIN 
 fprintf(stderr, outOfBoundariesError, returnResult);
 return returnResult; // or drop through
 }
 return returnResult;
}

FromIntegerToString()

With all the macros for portability, using 11 is wrong.

Better to scale the buffer based of INT_MIN or sizeof(int) and test for OOM. See ansi-c-maximum-number-of-characters-printing-a-decimal-int

sizeof(char) is always 1. Little point in using it here. If code might change from char to wchar_t, then better to use malloc(n * sizeof *convertedInputUnsignedInt);

// char *convertedInputUnsignedInt = malloc(11 * sizeof(char));
#define INT_STR_SIZE(type) (10*sizeof(type)*CHAR_BIT/33 + 3)
char *convertedInputUnsignedInt = malloc(INT_STR_SIZE(int));
if (convertedInputUnsignedInt == NULL) {
 Handle_OOM();
}
added 146 characters in body
Source Link
chux
  • 36.2k
  • 2
  • 43
  • 96

Just taking on 1 issue: overflow detection in FromStringToInteger()

UsingFirst, using a "bigger" integer type is not always available or neededand is not needed. long long may have the same range as int, so why use long long. Further, what if code was attempting to convert the maximum width integer type? Best to use a solution that works within the given type.

long long boundariesChecker; // not needed
int returnResult;

NegationSecond, negation is a problem when trying to createconvert a string to INT_MIN. Negating INT_MIN is UB when INT_MIN < -INT_MAX.

To solve this, test for overflow before multiplication and work use the negative side of int. Need a basic re-write of code.

Just taking on 1 issue: overflow detection in FromStringToInteger()

Using a "bigger" integer type is not always available or needed. long long may have the same range as int why use long long.

long long boundariesChecker;
int returnResult;

Negation is a problem when trying to create a string to INT_MIN. Negating INT_MIN is UB when INT_MIN < -INT_MAX.

To solve this, test for overflow before multiplication and work use the negative side of int. Need a basic re-write of code.

Just taking on FromStringToInteger()

First, using a "bigger" integer type is not always available and is not needed. long long may have the same range as int, so why use long long. Further, what if code was attempting to convert the maximum width integer type? Best to use a solution that works within the given type.

long long boundariesChecker; // not needed
int returnResult;

Second, negation is a problem when trying to convert a string to INT_MIN. Negating INT_MIN is UB when INT_MIN < -INT_MAX.

To solve this, test for overflow before multiplication and use the negative side of int. Need a basic re-write of code.

Source Link
chux
  • 36.2k
  • 2
  • 43
  • 96
Loading
lang-c

AltStyle によって変換されたページ (->オリジナル) /