Hello I wrote this function in C that takes a string of words and returns a two dimensional char array with each case initialized to a word in the right order, I compiled and it did the assigned task
char **decompose_string(char *string)
{
int i,j=0;
char* temp=malloc(sizeof(char*));
char **words_array=malloc((string_words_number(string)+1)*sizeof(char*)); //string_words_number return the number of words in string string
for(i=0;i<string_words_number(string);i++)
{
temp=NULL;
int l=0;
while(string[j]!=' ' && *string)
{
temp=realloc(temp,(l+1)*sizeof(char));
temp[l]=string[j];
j++;l++;
}
j++;
temp[l]='\0';
tab_mots=realloc(words_array,(string_words_number(string)+1)*sizeof(char)*(j-1));
words_array[i]=temp;
}
words_array[i]=NULL;
return words_array;}
And my main:
int main()
{
char* string1= "word1 and word2 and word3";
printf("Our initial string: %s\n",string1);
char** words_array1;
printf("After decomposition:\n");
words_array1=decompose_string(string1);
display_words_array(words_array1); //displays each element of the array in a line
words_array1=destroy_array(words_array1);
return 0;}
But as I executed the valgrind command to see weither there are any memory leaks this was the result:
==4648== Memcheck, a memory error detector
==4648== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==4648== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==4648== Command: ./test_csvl
==4648==
Our initial array: word1 and word2 and word3
After decomposition:
==4648== Invalid write of size 1
==4648== at 0x4009D9: decompose_string (in /home/omaima/2I001/TME3/test_csvl)
==4648== by 0x40078F: main (in /home/omaima/2I001/TME3/test_csvl)
==4648== Address 0x5204634 is 0 bytes after a block of size 4 alloc'd
==4648== at 0x4C2FD5F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4648== by 0x40097D: decompose_string (in /home/omaima/2I001/TME3/test_csvl)
==4648== by 0x40078F: main (in /home/omaima/2I001/TME3/test_csvl)
==4648==
==4648== Invalid read of size 1
==4648== at 0x4C30F74: strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4648== by 0x4EA969B: puts (ioputs.c:35)
==4648== by 0x400888: display_words_array (in /home/omaima/2I001/TME3/test_csvl)
==4648== by 0x40079F: main (in /home/omaima/2I001/TME3/test_csvl)
==4648== Address 0x5204634 is 0 bytes after a block of size 4 alloc'd
==4648== at 0x4C2FD5F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4648== by 0x40097D: decompose_string (in /home/omaima/2I001/TME3/test_csvl)
==4648== by 0x40078F: main (in /home/omaima/2I001/TME3/test_csvl)
==4648==
word1
and
word2
and
word3
==4648==
==4648== HEAP SUMMARY:
==4648== in use at exit: 8 bytes in 1 blocks
==4648== total heap usage: 30 allocs, 29 frees, 1,545 bytes allocated
==4648==
==4648== LEAK SUMMARY:
==4648== definitely lost: 8 bytes in 1 blocks
==4648== indirectly lost: 0 bytes in 0 blocks
==4648== possibly lost: 0 bytes in 0 blocks
==4648== still reachable: 0 bytes in 0 blocks
==4648== suppressed: 0 bytes in 0 blocks
==4648== Rerun with --leak-check=full to see details of leaked memory
==4648==
==4648== For counts of detected and suppressed errors, rerun with: -v
==4648== ERROR SUMMARY: 9 errors from 2 contexts (suppressed: 0 from 0)
I know that the missing free is the one for temp in the function but if I do free it I'll end up losing its value which I need for my array. So my question would be can I free it without losing its value ? or any other solution to guarantee both the functionality of my function and the erase of any memory leaks. Thank you for your time.
Sorry I forgot here's the function I used to free the allocated space:
char **destroy_words_array( char** words_array)
{
int i;
for(i=0;i<count_words(words_array);i++) free(words_array[i]); //wount_words return the number of elements of the array
free(words_array);
return words_array;
}
The memory leak isn't where you think it is. It happens at the start of your function:
char* temp=malloc(sizeof(char*));
char **words_array=malloc((string_words_number(string)+1)*sizeof(char*));
for(i=0;i<string_words_number(string);i++)
{
temp=NULL;
You allocate space for a pointer to temp
, but then you overwrite that pointer when you enter the for
loop. Remove that malloc
call and the leak goes away.
You have a bigger problem however, and that is reading and writing past the end of an allocated buffer. That's happening here:
temp=NULL;
int l=0;
while(string[j]!=' ' && *string)
{
temp=realloc(temp,(l+1)*sizeof(char));
temp[l]=string[j];
j++;l++;
}
j++;
temp[l]='\0';
Suppose the string in question has two characters to read. On the first iteration of your loop, l
is 0, so you allocate l+1 == 1
bytes to temp
. You then write to temp[l] == temp[0]
, which is fine, then you increment l
to 1. On the next iteration, you allocate l+1 == 2
bytes for temp
, write to temp[l] == temp[1]
, and increment l
to 2. Still good so far.
The problem is when you do temp[l]='\0';
outside of the loop. l
is now 2, and the size of the allocated memory is 2, so you're writing one element past the end of the array.
You need to allocate one more byte here:
j++;
temp=realloc(temp,(l+1)*sizeof(char));
temp[l]='\0';
Note also that you should be checking the return value of malloc
and realloc
throughout your code in case it fails.
Also, you're not correctly checking for the end of string
. You need to do:
while(string[j]!=' ' && string[j]!=0)