CODEWITHSUNDEEP
clinked-listvalgrind
Kernael
Kernael

Reputation: 61

C - Conditional jump or move depends on uninitialised value(s)

I'm getting this error from Valgrind when I try to run my program :

==23152== Conditional jump or move depends on uninitialised value(s)
==23152==    at 0x4C2D8D0: strcmp (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==23152==    by 0x40096C: str_lower_cmp (functions.c:41)
==23152==    by 0x400BB8: list_sort (list_sort.c:34)
==23152==    by 0x400CC7: get_wdir_content (working_dir.c:27)
==23152==    by 0x400C27: main (main.c:18)
==23152==  Uninitialised value was created by a heap allocation
==23152==    at 0x4C2C27B: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==23152==    by 0x400D4C: xmalloc (xfunctions.c:35)
==23152==    by 0x400886: lower_string (functions.c:20)
==23152==    by 0x400945: str_lower_cmp (functions.c:39)
==23152==    by 0x400BB8: list_sort (list_sort.c:34)
==23152==    by 0x400CC7: get_wdir_content (working_dir.c:27)
==23152==    by 0x400C27: main (main.c:18)
==23152== 
==23152== Conditional jump or move depends on uninitialised value(s)
==23152==    at 0x400BBB: list_sort (list_sort.c:34)
==23152==    by 0x400CC7: get_wdir_content (working_dir.c:27)
==23152==    by 0x400C27: main (main.c:18)
==23152==  Uninitialised value was created by a heap allocation
==23152==    at 0x4C2C27B: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==23152==    by 0x400D4C: xmalloc (xfunctions.c:35)
==23152==    by 0x400886: lower_string (functions.c:20)
==23152==    by 0x400945: str_lower_cmp (functions.c:39)
==23152==    by 0x400BB8: list_sort (list_sort.c:34)
==23152==    by 0x400CC7: get_wdir_content (working_dir.c:27)
==23152==    by 0x400C27: main (main.c:18)
==23152==

Though I can't say what is wrong, I'm guessing it comes from list_sort.c :

t_llist  *list_sort(t_llist *list)
{
    struct s_node *tmp;

    tmp = list->head;
    while (tmp != NULL)
    {
        if (tmp->next != NULL)
        {
            if (!tmp->name || !tmp->next->name)
                printf("Reached.\n");
            if (str_lower_cmp(tmp->name, tmp->next->name) > 0)
            {
                data_swap(tmp, tmp->next);
                tmp = list->head;
            }
            else
                tmp = tmp->next;
        }
        else
            return (list);
    }
    return (list);
}

Does that mean that at some point, the tmp->name or tmp->next->name value is uninitialised ?

EDIT (the functions.c code)

    char            *lower_string(char *s)
{
  char          *res;
  int           i;

  i = 0;
  res = xmalloc(sizeof(*res) * strlen(s) + 1);
  while (s[i])
    {
      if (s[i] >= 'A' && s[i] <= 'Z')
        res[i] = s[i] + 32;
      else
        res[i] = s[i];
      i++;
    }
  s[i] = '\0';
  return (res);
}

int             str_lower_cmp(char *s1, char *s2)
{
  char          *tmp1;
  char          *tmp2;
  int           res;

  tmp1 = lower_string(s1);
  tmp2 = lower_string(s2);
  res = strcmp(tmp1, tmp2);
  free(tmp1);
  free(tmp2);
  return (res);
}

Upvotes: 3

Views: 3462

Answers (3)

Serve Laurijssen
Serve Laurijssen

Reputation: 9763

when the "char *s" passed to lower_string is an empty string you'd have a crashing program too. Calling calloc as jcm said would help fix that problem

Upvotes: 0

jcm
jcm

Reputation: 2578

Initially valgrind is telling you that you're running strcmp with a memory address allocated with malloc, coming from function lower_string, but that has no initial value assigned.

This would mean an undefined behavior, meaning that, depending on your code, can be very dangerous because can lead to unexpected results.

I would suggest using calloc in lower_string.

Edit: your're setting s[i] to 0 instead of res[i] (the pointer you've allocated and returning). On the other hand, I would suggest using calloc and checking res!=NULL

Upvotes: 4

WhozCraig
WhozCraig

Reputation: 66254

Your error is here in lower_string You're not terminating the string you're allocating:

char *lower_string(char *s)
{
    char *res;
    int i;

    i = 0;
    res = xmalloc(sizeof(*res) * strlen(s) + 1);
    while (s[i])
    {
        if (s[i] >= 'A' && s[i] <= 'Z')
            res[i] = s[i] + 32;
        else
            res[i] = s[i];
        i++;
    }
    s[i] = '\0'; // THIS IS WRONG
    return (res);
}

The marked line should be this:

    res[i] = '\0'; // THIS IS RIGHT

And note, this would be caught if you properly passed the input string as a const parameter:

char *lower_string(const char *s) // MAKE PARAM CONST

Doing so would have failed to compile because your s[i] = '\0' assignment would have violated the const-condition. General rule, unless you need to modifying something passed as a a by-address parameter, make it const

Upvotes: 2

Related Questions