Runtime error: reading uninitialized value, how can I fix it?

Question

This function is basically just supposed to compare 2 strings and return their ASCII difference if they are different. It works perfectly fine when I compile it with the GCC compiler, but when I run it through the online compiler that is used to upload our classes homework, I get this error message:

Error near line 98: Reading an uninitialized value from address 10290

Line 98 is marked in the below code. I am not quite sure what the problem is and how I'm supposed to fix it. Does anyone have an idea?

int stringCompare(char * pStr1, char * pStr2)  {
    int n = 100;
    int difference;

    for (int i = 0; i < n; i++)  {
        difference = pStr1[i] - pStr2[i]; // line 98
        if (difference != 0)  {
            return difference;
        }
    }
    return difference;
}

Please don't post text errors as images or links to images - [reasoning](https://stackoverflow.com/help/minimal-reproducible-example). And you MUST post a [minimal verifiable example](https://stackoverflow.com/help/minimal-reproducible-example). The root cause is likely not in the code you have show - more likely the problem is in the setting of `pStr1` and/or `pStr2`. — kaylum, Apr 11 '20 at 01:03
[It might look confusing because that is not a compiler error, but rather a runtime warning emitted by I don't precisely know what, maybe your IDE? In any case it's telling you that values in `pStr1` *or* `pStr2` (or both) are not initialized and you should explicitly initialize them before calling the function or doing anything else with the values.](https://stackoverflow.com/questions/201101/how-to-initialize-all-members-of-an-array-to-the-same-value) — Marco Bonelli, Apr 11 '20 at 01:04
ok, sorry about the image, will remember that for the future. The function shown is the only thing that I had to code myself. Everything else is already preset by the exercise. — seal9055, Apr 11 '20 at 01:06
Regardless you still need to post the MVE. We need to see what the input is. For example, the input may be an array less than `100` which could cause that warning because you always assume an array of 100. — kaylum, Apr 11 '20 at 01:07
yeah, the array is definitely less than 100. I just used 100 since I wanted to have an upper bound that is longer than any input would be. — seal9055, Apr 11 '20 at 01:09
Well that's your problem then. You must not traverse past the end of the array. If those are strings as implied then end the loop based on NUL terminator of either stirng. — kaylum, Apr 11 '20 at 01:10

score 0 · Answer 1 · answered Apr 11 '20 at 01:05

0

Your code can skip over EOLN, if string equals, and try to compare memory after end of lines. To fix this, you need instantly return, if both string equals, and you see EOLN char '\0' in both strings at position i. Try my fix:

int stringCompare(char * pStr1, char * pStr2)  {
    int n = 100;
    int difference;

    for (int i = 0; i < n; i++)  {
        difference = pStr1[i] - pStr2[i]; 
        if (difference != 0 || pStr1[i] == '\0')  {
            return difference;
        }
    }
    return difference;
}

answered Apr 11 '20 at 01:05

olegarch

3,670
1
20
19

`\0` is the NUL character, which marks the end of the string (not the end of a line). EOLN is just a concept and does not exist as a character. – Marco Bonelli Apr 11 '20 at 01:07
In my text, I mentioned EOLN as abbreviation End-Of-Line. as you see, I did not compare anything with EOLN within my fix. – olegarch Apr 11 '20 at 01:10
1

Well I sure see that since `EOLN` does not exist in C. Nonetheless, calling `\0` "EOLN" is wrong. – Marco Bonelli Apr 11 '20 at 02:18
Disagree. In C, a string is terminated with `\0` - this is standard from K&R. My code, returns diff != 0, if it see `\0` in the any of string, while other does not have '\0' at same position, so will stop at 1st `\0` in either string. Or, if both strings equals, it return diff == 0, because of str1[i] ==0 (and this assume, str2[i] is also == 0). If you disagree - please, provide pair of strings, where is my code walk over '\0' and continue comparing undefined memory. – olegarch Apr 12 '20 at 00:23
Ok scrap my comments you're totally right I'm stupid. Sorry about that :\ – Marco Bonelli Apr 12 '20 at 13:00

Marco Bonelli · Answer 2 · 2020-04-11T19:13:40.337

The problem in your code is that you fail to check the real length of the strings before indexing them. You are iterating with i from 0 to 99, but you do not check for the NUL terminator (\0) that marks the end of a string and therefore your for loop goes beyond the end of the string resulting in undefined behavior accessing memory that is not supposed to (which is what the error is telling you).

The correct way to iterate over a string, is not to loop a fixed amount of cycles: you should start from index 0 and check each character of the string in the loop condition. When you find \0, you stop. See also How to iterate over a string in C?.

Here's a correct version of your code:

int stringCompare(char *pStr1, char *pStr2) {
    size_t i;

    for (i = 0; pStr1[i] != '\0' && pStr2[i] != '\0'; i++)  {
        if (pStr1[i] != pStr2[i])
            break;
    }

    return pStr1[i] - pStr2[i];
}

You could even write this more concisely with a simple while loop:

int stringCompare(char *pStr1, char *pStr2) {
    while (*pStr1 && *pStr1 == *pStr2) {
        pStr1++;
        pStr2++;
    }

    return *pStr1 - *pStr2;
}

Of course, both the above functions expect two valid pointers to be passed as arguments. If you also want to allow invalid pointers you should check them before starting the loop (though it does not seem like you want to do that from your code).

Runtime error: reading uninitialized value, how can I fix it?

2 Answers2