Conditional jump or move depends on uninitialised value(s) at strcpy

Question

Valgrind detects a problem with strcpy in the following code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main () {

    char **array;
    int mallocedLen = 15;
    int arrLen = 10;
    char tempArr[20][20] = {"abc", "123", "def", "456", "ghi", "789"};
    int ii;
    
    array = (char**)malloc(sizeof(char*) * arrLen);
    for(ii = 0; ii < arrLen; ii++){
        array[ii] = (char*)malloc(sizeof(char) * (strlen(tempArr[ii]) + 1));
        strcpy(tempArr[ii], array[ii]);
        array[ii][strlen(tempArr[ii])] = '\0';
        mallocedLen++;
    }
    
    return 0;
}

==4360== Conditional jump or move depends on uninitialised value(s)

==4360== at 0x483F0A2: strcpy (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)

The code seems to compile fine so I don't think it's a problem with mallocing.

Answer 1

Here:

 array[ii] = (char*)malloc(sizeof(char) * (strlen(tempArr[ii]) + 1));
 strcpy(tempArr[ii], array[ii]);

You allocate space to array[ii], then immediately pass it as a source string to strcpy() when it does not yet contain a valid string.

It seems likely that you intended tempArr[ii] as the source:

strcpy( array[ii], tempArr[ii] ) ;

And the nul termination is unnecessary - strcpy() does that already.

Conditional jump or move depends on uninitialised value(s)

Means what it says. For example strcpy() might have:

while( *source != 0 )
{
    ...
}

But *source has not been initialised, so the loop may or may not iterate.

Note also that had you declared tempArr as:

const char* tempArr[] = {"abc", "123", "def", "456", "ghi", "789"}; 

the compiler would have caught your error. I assume tempArr is immutable? In which case declare it as such. It is always better to trap semantic errors at build time rather then rely on runtime detection tools. It is also more memory efficient in this case.

Another issue is :

for(ii = 0; ii < arrLen; ii++){

where arrlen is 10 but tempArr has only 6 initialisers. Better to use a sentinal value such as NULL or an empty string. Or if you use the const char* tempArr[] declaration above, then:

const char* tempArr[] = {"abc", "123", "def", "456", "ghi", "789"}; 
const int arrLen = sizeof(tempArr) / sizeof(*tempArr) ;

Answer 2

It's strcpy(destination, source). Also strcpy() 0-terminates the copy.

So replace

  strcpy(tempArr[ii], array[ii]);
  array[ii][strlen(tempArr[ii])] = '\0';

by

  strcpy(array[ii], tempArr[ii]);