Search code examples
cvalgrind

Valgrind Conditional jump or move depends on uninitialised value(s) - all variables initiatlized


Valgrind Conditional jump or move depends on uninitialised value(s)

I have a function that removes excess whitespace from a dynamically allocated string, it works fine but I get a valgrind error concerning uninitialized variables and I can't seem to pin point where it is coming from. I have all variables initialized but that doesnt resolve the error.

Here's my code below

#include "main.h"

void normalize_wspace2(char *_str);

int main(void)
{
    char s[] = " ls       -la f1\t folder2 this one\t\t    ";

    printf("s initial = %s,\n", s);
    normalize_wspace2(s);
    printf("s normalized = %s,\n", s);
}
void normalize_wspace2(char *_str)
{
    size_t i = 0, j = 0;

    for (i = 0, j = 0; _str[i] != '\0'; j++, i++)
    {
        if (_str[i] == ' ' || _str[i] == '\t' || _str[i] == '\v')
        {/* if whitespace is matched */
            if (i != 0)
            {/*only copy a single ' ' to normalized string*/
                _str[j] = ' ';
                i++;
                j++;
            }
            while (_str[i] == ' ' || _str[i] == '\t' || _str[i] == '\v')
            {/* loop through and ignore all other spaces*/
                i++;
            }
        }
        _str[j] = _str[i]; /*copy non-whitespace char to normalized string */
    }
    _str[j] = '\0';
    del_twspace(_str);
}

void del_twspace(char *s)
{
    size_t i = 0;
    int last_char = -1;

    if (s == NULL)
    {
        perror("error: del_twspace passed null\n");
        return;
    }
    while (s[i] != '\0')
    {/* find the last char, ignoring any whitespace */
        if (s[i] != ' ' && s[i] != '\t' && s[i] != '\n')
            last_char = i;
        i++;
    }
    
    s[last_char + 1] = '\0'; /* set char after last char to null */
}

Valgrind output


$ valgrind -s --leak-check=full --show-leak-kinds=all --track-origins=yes ./a.out

==8876== Memcheck, a memory error detector
==8876== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==8876== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==8876== Command: ./a.out
==8876== 
==8876== Conditional jump or move depends on uninitialised value(s)
==8876==    at 0x109323: normalize_wspace2 (in /workspaces/103507869/functions/cln/a.out)
==8876==    by 0x1091F4: main (in /workspaces/103507869/functions/cln/a.out)
==8876==  Uninitialised value was created by a stack allocation
==8876==    at 0x109169: main (in /workspaces/103507869/functions/cln/a.out)
==8876== 
s initial =  ls       -la f1     folder2 this one           ,
s normalized = ls -la f1 folder2 this one ,
==8876== 
==8876== HEAP SUMMARY:
==8876==     in use at exit: 0 bytes in 0 blocks
==8876==   total heap usage: 1 allocs, 1 frees, 4,096 bytes allocated
==8876== 
==8876== All heap blocks were freed -- no leaks are possible
==8876== 
==8876== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
==8876== 
==8876== 1 errors in context 1 of 1:
==8876== Conditional jump or move depends on uninitialised value(s)
==8876==    at 0x109323: normalize_wspace2 (in /workspaces/103507869/functions/cln/a.out)
==8876==    by 0x1091F4: main (in /workspaces/103507869/functions/cln/a.out)
==8876==  Uninitialised value was created by a stack allocation
==8876==    at 0x109169: main (in /workspaces/103507869/functions/cln/a.out)
==8876== 
==8876== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Solution

  • The problem occurs with trailing whitespace: After this loop

            while (_str[i] == ' ' || _str[i] == '\t' || _str[i] == '\v')
            {/* loop through and ignore all other spaces*/
                i++;
            }
    

    i will already be the index of the trailing '\0' in _str. Then it's incremented again by for() and so the next check for _str[i] != \0' is already beyond the bounds of _str.

    Do solve its you could just add

            if (_str[i] == '\0' ) {
                _str[j] = '\0';
                break;
            }
    

    after the while loop or you change the loop body to

        if (_str[i] == ' ' || _str[i] == '\t' || _str[i] == '\v')
        {/* if whitespace is matched */
            if (i != 0)
            {/*only copy a single ' ' to normalized string*/
                _str[j] = ' ';
                 j++;
           }
            while (_str[i+1] == ' ' || _str[i+1] == '\t' || _str[i+1] == '\v')
            {/* loop through and ignore all other spaces*/
                i++;
            }
        } else {
            _str[j] = _str[i]; /*copy non-whitespace char to normalized string */
            j++;
        }
    

    A completely different solution which avoids writing to i within the for() loop could look like that (btw: I remove leading _ here, because these are resevered names:

        bool whitespace = false;      
        int j = 0;
        for( int i=0; str[i] != '\0'; ++ ) {
             if( isspace(str[i]) ) { // too lazy to write whitespace characters 
                 if( j > 0 ) {
                     whitespace  = true;
                 }
             } else {
                 if( whitespace ) {
                     str[j++] = ' ';
                     whitespace = false;
                 }
                 str[j++] = str[i];
             }
        }
        str[j] = '\0';