Let's get better in C - Exercise #4 - What's the problem with the following code?

Question

Instructions

This exercise is for anyone who would like to get better in C/C++ coding.

I've provided some sample code, that has two problems with it. (At least two, that I wanted to focus on in this exercise, but accidentally I could have made more mistakes. XD)

Let's see who finds both of the problem.

Please, tell me (for each problem) that

1. what part of the code has a problem
2. why it is a problem
3. how would you fix it

The Code (with the two problems):

#include <stdio.h>  // included for: printf
#include <string.h> // included for: strlen
#include <ctype.h>  // included for: isspace

// Remove all white space characters from the tail of the string
// str: the string to trim; cannot be NULL
void trimr(char * str)
{
    unsigned int len = strlen(str) - 1;     // Skip the terminating zero
    while (isspace(str[len]) && len >= 0)   // Skip all whitespace characters
        len--;

    str[len + 1] = '\0';                    // Place a terminating zero after the first non-white space character.
}

int main()
{
    char test[] = "This is a test.    ";

    printf("[%s] -> ", test);
    trimr(test);
    printf("[%s]\n", test);

    return 0;
}

Sample execution:

[This is a test.    ] -> [This is a test.] 

Good luck! ;)

Answer 1

The QA quote here springs to mind -> "A QA engineer walks into a bar. Orders a beer. Orders 0 beers. Orders 99999999999 beers. Orders a lizard. Orders -1 beers. Orders a ueicbksjdhd"

The first error is when you throw an empty cstring into the function, strlen will return 0, and len will be assigned to 0 - 1 which is the maximum value of an unsigned integer. Given str is a char pointer, the assignment of str[len+1] will be amending memory of something unintended. Conversely, if a cstring with a size greater than a value of unsigned int's maximum value is entered the function will not be correctly checking, as the cstring is greater than that of the value, and by definition of the strlen function a long can be returned which is a 64 bit unsigned integer, it will be checking for spaces midway through the cstring and not from the end.

To fix, I would probably do the following

uinsigned int len = strlen(str);

if (strlen == 0) return;

if (len == UINT32_MAX) // do uint64_t code

else  // do your code body alongside --len at the start

Although I would probably put a return in the body of the second if code and get rid of the else statement. Or you could avoid the need to check for a cstring with length > maximum uint32 size by using a uint64_t from the start.

Secondly, if the cstring given is a single whitespace, or only contains white spaces, the program will decrement len to the max value of uint32 again, accessing memory it shouldn't. You could fix by just making the len size check in the while loop to be len > 0 instead of >=, but then you'd need to check if str[len] is a space again and make str[len] = '/0' instead of str[len+1], or find an alternative solution for that occurence

Thirdly, if a nullptr is given to the function the program compiles and runs, but appears to break or have no output. I'm not sure why this is. Although you mention str cannot be null in the comments, it can still accept a nullptr. To fix you simply add an if (str == nulptr) return; to the top of the function.

I'd also be using uint32_t / 64_t version of len assignment/decleration instead of unsigned int just to be more concise with the size of integer.

Comments

Yes, indeed. The problem is with over/underflow.

(To be fair, I wanted to use size_t, but for many, it's not clear what the type is so I used unsigned int, but unsigned long should have been a better choice.)

For NULL, your correct, thought I added the comment so people wouldn't focus on the NULL but rather try to find the over/underflow problems.

Well done! :)

Answer 2

I have identified the issues(,but giving pointers (no pun intended :) ) to where code can go wrong,please think and answer .

• does the unsigned int data type support -ve values ?? or what will the result of decrementing it ,when the current value is 0!!
• If the above question is conquered , think how empty strings need to be handled.
• Now that you have made it this far, check how strings with single space char needs to be handled.

Comments

Good tips. ;)

Answer 3

There's plenty of help from xDELLx.

Is there anyone willing to give a solution (fix) for the problem? :)