0
votes

So, my goal was to define a struct in which there is -

  1. A command name (e.g. - "print")
  2. Command arguments counter
  3. A strings array containing the arguments.

You can review my code, but I'm really having a hard time understanding what am I doing wrong -

  1. I use malloc to dynamically set my_struct.command size
  2. I use malloc to dynamically set my_struct.arguments array size
  3. I use realloc to dynamically increase my_struct.arguments size for every argument I set
  4. I use malloc to dynamically set my_struct.arguments[i] size
  5. I finally call cleanup(), to free any dynamically assigned pointers.

I keep getting LOTS of memory leaks. But I cannot understand why.

Help and tips will be kindly appreciated.

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

struct  {
    char *command;
    int arguments_count;
    char **arguments;
} my_struct;

void cleanup(void);

int main() {
    int i;

    my_struct.command = (char *)malloc(6*sizeof(char));

    my_struct.command = "print";
    my_struct.arguments_count = 1;
    my_struct.arguments = (char **)malloc(sizeof(char *));

    my_struct.arguments[0] = "hello";

    for(i = 1 ; i < 10; i++) {
        my_struct.arguments = (char **)realloc(my_struct.arguments, sizeof(char *)*(i+1));
        my_struct.arguments[i] = (char *)malloc(8*sizeof(char));
        my_struct.arguments[i] = "hello";
        my_struct.arguments_count++;
    }

    printf("Arguments count is: %d\n", my_struct.arguments_count);
    printf("The arguments are:\n");

    for(i = 0; i < 10; i++) {
        printf("%s\n", my_struct.arguments[i]);
    }

    cleanup();

    exit(0);
}

void cleanup(void) {
    int i;

    for(i = 0; i < 10; i++)
        free(my_struct.arguments[i]);

    free(my_struct.arguments);
    free(my_struct.command);
}
1
my_struct.command = (char *)malloc(6*sizeof(char)); my_struct.command = "print"; That right there is a leak!Kevin
And please for the love of god don't use sizeof(char)vmt
arg[i] = malloc(8*sizeof(char)); arg[i] = "hello"; -- That's a memory leak. You reserve memory, but immediately lose the handle to it by assigning to a string literal. The string literal isn't something you can free later, either.M Oehm
strcpy to copy a string to a char array.Matheus Rossi Saciotto
strcpy(my_struct.command, "hello");Matheus Rossi Saciotto

1 Answers

0
votes

strdup - The strdup() function returns a pointer to a new string which is a duplicate of the string s. Memory for the new string is obtained with malloc, and can be freed with free.

my_struct.command = strdup("print");

my_struct.arguments_count = 1;
my_struct.arguments = (char**) malloc(sizeof(char*));
my_struct.arguments[0] = strdup("hello");

for (int i=1; i < 10; ++i) {
    // if the number of args is known, allocate before entering the loop
    my_struct.arguments = (char**) realloc(my_struct.arguments, sizeof(char*)*(i+1));
    my_struct.arguments[i] = strdup("hello");
    my_struct.arguments_count++;
}

// in your cleanup use the arguments_count var instead of the literal 10
for (int i=0; i < my_struct.arguments_count; ++i)

Your mistake was:

// allocate a memory block of 6 bytes
// assign the address of that block to command
my_struct.command = malloc(6);

// then you assigned the address of the string 'print' to command
// therefore the previous allocated block is lost -> mem leak
my_struct.command = "print";

// strdup does the following
return memcpy(malloc(strlen(str) + 1), str, strlen(str) + 1);