Valgrind leak of memory in a function that concatenates strings

Question

I using valgrind to find a leak of memory. I wrote a function "prefix_to_string" to concatenate any two strings, but when I use the command

valgrind --leak-check=full ./helloworld

it says that I have a lot of leaks of memory. I really don't know where and why. I asked a friend why it was happening and he says that it was for doing malloc 2 times, 1 out the function and 1 in the function, but I don't know how to take care of that leak, because I think that I need to do those memory requests. Here is the output that Valgrind gives me:

==9078== Memcheck, a memory error detector
==9078== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==9078== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==9078== Command: ./holamundo
==9078==
==9078== error calling PR_SET_PTRACER, vgdb might block
150:62
bye
==9078==
==9078== HEAP SUMMARY:
==9078==     in use at exit: 63 bytes in 4 blocks
==9078==   total heap usage: 5 allocs, 1 frees, 575 bytes allocated
==9078==
==9078== 5 bytes in 1 blocks are definitely lost in loss record 1 of 4
==9078==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9078==    by 0x400740: prefix_to_string (in /mnt/c/Users/MrRaul/desktop/Tareas_edd/test_en_C/test_informales/holamundo)
==9078==    by 0x4008AC: main (in /mnt/c/Users/MrRaul/desktop/Tareas_edd/test_en_C/test_informales/holamundo)
==9078==
==9078== 7 bytes in 1 blocks are definitely lost in loss record 2 of 4
==9078==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9078==    by 0x400740: prefix_to_string (in /mnt/c/Users/MrRaul/desktop/Tareas_edd/test_en_C/test_informales/holamundo)
==9078==    by 0x4008C3: main (in /mnt/c/Users/MrRaul/desktop/Tareas_edd/test_en_C/test_informales/holamundo)
==9078==
==9078== 8 bytes in 1 blocks are definitely lost in loss record 3 of 4
==9078==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9078==    by 0x400740: prefix_to_string (in /mnt/c/Users/MrRaul/desktop/Tareas_edd/test_en_C/test_informales/holamundo)
==9078==    by 0x4008D8: main (in /mnt/c/Users/MrRaul/desktop/Tareas_edd/test_en_C/test_informales/holamundo)
==9078==
==9078== 43 bytes in 1 blocks are definitely lost in loss record 4 of 4
==9078==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9078==    by 0x400897: main (in /mnt/c/Users/MrRaul/desktop/Tareas_edd/test_en_C/test_informales/holamundo)
==9078==
==9078== LEAK SUMMARY:
==9078==    definitely lost: 63 bytes in 4 blocks
==9078==    indirectly lost: 0 bytes in 0 blocks
==9078==      possibly lost: 0 bytes in 0 blocks
==9078==    still reachable: 0 bytes in 0 blocks
==9078==         suppressed: 0 bytes in 0 blocks
==9078==
==9078== For counts of detected and suppressed errors, rerun with: -v
==9078== ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 0 from 0)

and here is the main of my code, so this way you can reproduce the problem:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h>
#include <unistd.h>
#include <string.h>
#include <math.h>

char* prefix_to_string(char* first_string,char* second_string){
    char* name = first_string;
    char* extension = second_string;    
    char* name_with_extension;
    name_with_extension = malloc(strlen(name)*(sizeof(char))+strlen(extension)*(sizeof(char))+1); /* make space for the new string (should check the return value ...) */
    strcpy(name_with_extension, name); /* copy name into the new var */
    strcat(name_with_extension, extension); /* add the extension */
    return name_with_extension;
}

static char *itoa_simple_helper(char *dest, int i) {
  if (i <= -10) {
    dest = itoa_simple_helper(dest, i/10);
  }
  *dest++ = '0' - i%10;
  return dest;
}

char *itoa_simple(char *dest, int i) {
  char *s = dest;
  if (i < 0) {
    *s++ = '-';
  } else {
    i = -i;
  }
  *itoa_simple_helper(s, i) = '\0';
  return dest;
}

int main(int argc, char *argv[]) {

int idx = 150;
int id = 62;
char str_idx[20];
char str_id[20];
itoa_simple( str_idx ,idx);
itoa_simple( str_id,id);
char *text_to_write;
text_to_write = malloc(2+sizeof(str_id)+sizeof(str_idx)+1);
text_to_write = prefix_to_string(str_idx,":");
text_to_write = prefix_to_string(text_to_write,str_id);
text_to_write = prefix_to_string(text_to_write,"\n");

printf("%s",text_to_write);
printf("bye\n");
free(text_to_write);




return 1;
}

Answer 1

You don't call free() often enough — you can't expect to avoid memory leaks if you don't call free() to release each separate memory allocation. And your repeated assignments to text_to_write in main() mean that you discard the only pointers to some of the allocated memory, so you can't free what was allocated. C requires endless care with memory management.

You have:

char *text_to_write;
text_to_write = malloc(2+sizeof(str_id)+sizeof(str_idx)+1);
// This assignment throws away the memory from the previous malloc
text_to_write = prefix_to_string(str_idx,":");
// This assignment throws away the memory from the previous prefix_to_string
text_to_write = prefix_to_string(text_to_write,str_id);
// This assignment also throws away the memory from the previous prefix_to_string
text_to_write = prefix_to_string(text_to_write,"\n");

printf("%s",text_to_write);
printf("bye\n");
// Calling free here only releases the last allocation from prefix_to_string
free(text_to_write);

You need something more like:

char *part1 = prefix_to_string(str_idx, ":");
char *part2 = prefix_to_string(part1, str_id);
char *part3 = prefix_to_string(part2, "\n");

printf("%s", part3);
printf("bye\n");

free(part1);
free(part2);
free(part3);