free allocated memory in function-CodePudding

i have function which is written in c ,in this function i allocate 2 string as temp and found, but i cant free temp string. i think it may due to using of temp in result array. can any one helps me. here is the function.

void split(char* input, char* delim, char** result,int size) {

    char* tmp = malloc((strlen(input)) * sizeof(char));
    char* found = malloc((strlen(input)) * sizeof(char)); 
    
    tmp=strcpy(tmp, input);

    // #pragma omp parallel for
    for (int i=0; i<size; i  ) {
        found = strstr(tmp, delim);
        if (found != NULL) {

            int length = found - tmp;
            result[i]=malloc((length 1) * sizeof(char));
            result[i] = strncpy(result[i], tmp, length);
            *(result[i]   length) = '\0';
            tmp = found   strlen(delim);

        } else {

            result[i]=malloc(strlen(tmp) * sizeof(char));
            result[i] =strncpy(result[i], tmp, strlen(tmp));
  
        }
    }

    // free(tmp);
    free(found);
}

here size is number of sub strings after split

when i remove the comment of this line: // free(tmp); then this err occurs:

munmap_chunk(): invalid pointer
Aborted (core dumped)

can i ask you to help me for writing correct split function

CodePudding user response：

You do assignments to tmp. That means the pointer tmp might no longer point to the same location that malloc returned.

You need to pass the exact same pointer to free that was returned by malloc.

You have the same problem with found, you assign to it and possible change where it points.

Passing an invalid pointer to free leads to undefined behavior.

You also have another problem: You go out of bounds of the original memory allocated and pointed to by tmp. That's because you seem to have forgotten that strings in C are really called null-terminated strings.

When you allocate memory for a string, you need to include space for the null-terminator at the end. And it's not counted by strlen.

Going out of bounds of allocated memory also leads to undefined behavior.

CodePudding user response：

The function does not make a sense.

For starters it invokes undefined behavior

char* tmp = malloc((strlen(input)) * sizeof(char));
char* found = malloc((strlen(input)) * sizeof(char)); 

tmp=strcpy(tmp, input);
//...

because you allocated to enough memory to store the terminating zero character '\0' of the string input in the character array tmp.

Secondly the function has a memory leak because at first memory was allocated and its address was assigned to the pointer found and then the pointer found was reassigned in the call of strstr in the for loop.

char* found = malloc((strlen(input)) * sizeof(char)); 

//...

// #pragma omp parallel for
for (int i=0; i<size; i  ) {
    found = strstr(tmp, delim);
    //...

So the address of the early allocated memory is lost and the memory can not be freed.

And this for loop

for (int i=0; i<size; i  ) {

is just senseless.

You may not call free neither for tmp nor for found. The pointer found does not point to a dynamically allocated memory and the pointer tmp is being changed within the for loop.