Skip to main content
Code Review

Return to Revisions

4 of 4
Commonmark migration

Issues Concerning Allocation

It is not necessary to cast the result of a call to malloc() in C, and many consider this to be a more error-prone style. It is easier to write correctly, read, and maintain the allocation line if you write it like this:

char *numberAsString = malloc(sizeof(*numberAsString));

This way, if the type is changed at some point in the future, there is only one place you need to update, versus three places to update in your version.

I would also suggest moving the allocation from main() into the intToStr() function (which is already reallocating the memory), and returning a pointer to the allocated memory to the calling function. This would necessitate changing the function signature of intToStr() to:

char * intToStr(int myNumber, char* myString);

Then in main(), you should declare:

char *numberAsString = NULL;

The same advice given above for malloc() applies to the calls to realloc() in the function intToStr(). Additionally, realloc() returns a NULL pointer in the event of an error. By assigning the return value directly to myString you risk leaking memory by losing the pointer to the previous allocation. You should store the return value of realloc() in a temporary variable, and then assign it to myString if it is non-null.

Continuing with allocation issues, I would consider mallocing enough memory to hold any int number-string (really, I would suggest using an int32_t number-- see below) at the beginning of intToStr(). Then, before returning the pointer to this memory, you can realloc() to the size of the actual number-string, i.e.:

number_string_sz = 12; // for int32_t plus space for `-` and `0円`
myString = malloc(number_string_sz);
...
temp = realloc(myString, strlen(myString) + 1);
if (temp) {
 myString = temp;
}

You may wish to return a NULL pointer from intToStr() if there is an allocation error, and let the caller handle the error. Of course, the caller is responsible for freeing allocated memory.

Integer Overflow

Note that it is possible that myNumber is equal to INT_MIN, and then the line myNumber *= -1; would lead to integer overflow (invoking undefined behavior for signed integer types). Consider using a fixed-width type, such as int32_t, that is passed into the function, and copy this to a larger fixed-width type (int64_t) for a working copy within the function. The reasoning here is that the widths of the integer types are implementation-dependent, and even long int is only guaranteed to be at least as wide as int. By choosing specific widths for your integer types, you can protect against integer overflow in your calculations. The int64_t type is larger than int32_t, so the smallest value which can be held by an int32_t, when stored in an int64_t, can safely be multiplied by -1.

ad absurdum
  • 894
  • 1
  • 10
  • 21
default

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