-1

I get the following error reported by Valgrind:

==19634== Conditional jump or move depends on uninitialised value(s)
==19634==    at 0x4C2E2D0: __strcmp_sse42 (vg_replace_strmem.c:852)
==19634==    by 0x400908: main

For what I assume is the use of strcmp. However, I do not know what unintialised values are used as I set values for both inputs. here is a snippet of code where I show you the values are not unintialised:

 53         strncpy(cmp1,sub,k-1); // first k - 1 of the new substring
 54         strncpy(cmp2,last->key + 1,k-1); // last k -1 of the previous/last substring
 55         if (strcmp(cmp1,cmp2) == 0) 

I assign values to both cmp1 and cmp2 before, and then use strcmp. So, I am wondering what the issue is?

Edit: Please let me know if you would like more code to be shown.

  • 2
    `Please let me know if you would like more code to be shown.` I let you know. Please show so much code as it is needed, for others to compile your code on their computer and run valgrind and get the same warning. Ie. create an [MCVE] – KamilCuk Aug 02 '20 at 20:00
  • Did you allocate memory for `cmp1` and `cmp2`? – Tibrogargan Aug 02 '20 at 20:03
  • Just because you copy values into cmp1 and cmp2, doesn't mean they're initialized. The values where they come from might be uninitialized, and Valgrind tracks this. What does `--track-origins=yes` say? https://stackoverflow.com/a/2612524/530160 – Nick ODell Aug 02 '20 at 20:03
  • @Tibrogargan no I did not. But the error is fixed now from the answer below. – lameprogrammer01 Aug 02 '20 at 20:15

1 Answers1

1

strncpy doesn't ensure the resulting (dest) string is null-terminated, and you don't put any NUL character (\0) at the end of both copied strings. Therefore strcmp goes past the end of the strings, past the memory you allocated for them.

One way to fix this is to add the terminating NUL character before calling strcmp:

cmp1[k-1] = 0;
cmp2[k-1] = 0;

Of course I assume you have enough room ta add that terminating NUL character.

Another, and better, way is to use strncmp instead of strcmp:

if (strncmp(cmp1, cmp2, k-1) == 0)

This way, you don't have to copy strings to compare them:

if (strncmp(sub, last->key + 1, k-1) == 0)
xhienne
  • 5,738
  • 1
  • 15
  • 34
  • How would I ensure that there is NULL character at the end in that case to prevent this from happening? Thank you btw – lameprogrammer01 Aug 02 '20 at 20:08
  • @lameprogrammer01 Answer amended – xhienne Aug 02 '20 at 20:11
  • thank you. Could you further elaborate on this issue ? I was completely unaware that this was a thing - is it a common issue? By setting that last space to NULL, am I going to affect other parts of the program? – lameprogrammer01 Aug 02 '20 at 20:14
  • In C, 0 signals the end of a string. If you omit that ending 0, string-manipulating functions won't have any idea where the strings end and will go past the end of them, reading memory areas they are not supposed to read. Use `strncmp` (see my answer) – xhienne Aug 02 '20 at 20:23