Freeing alloated memory in a function called in another loop

Question

am trying to free memory allocated in a user defined function. Am planning on running the code on an embedded device, STM32F303k8 which has 64Kb of Flash and 16KB of SRAM. I havent tried out the code but i fear its not going to do what it¨s supposed to do. Its going to run out of memory because of failure to deallocate the memory assigned

I have tried to free the memory in my custom program called split. However, it doesnt even compile and always crashes at the free() function.

        //This happens somewhere in the main function     
        // str_gprmc is a string thats trying to be split 
        // "$GPRMC,130133.00,A,5741.6029848,N,01158.3855831,E,11.522,170.0,270319"

    for (int k = 0; k < ARRAY_SIZE(str_gprmc); k++)
    {
        char **arr = NULL;

        // Now do the splitting please
        split(str_gprmc[k], ',', &arr);
    }´

        // and the split function
int split(const char *ptr_original_string, char delimiter, char ***ptr_main_array)
{
    // This variable holds the number of times we think we shall iterate through our array once its split
    int count = 1;

    // This variable holds the number of characters we have to iterate through for each split string
    int split_string_len = 1;

    // This variable helps us iterate through collections
    int i = 0;

    // Points to the first character of the whole string
    char *ptrTemp_string_holder;

    // Points to the first character of a split string from the main string 
    char *t;

    ptrTemp_string_holder = ptr_original_string;

    // First we count the number of times the delimiter is occurring in the string we want to split
    // Iterate through the string until we reach either the Courage Return character CR : '\r', the Line Feed LF : '\n' or the NULL : '\0'
    while (*ptrTemp_string_holder != '\0')
    {
        if (*ptrTemp_string_holder == delimiter)
            count++;
        ptrTemp_string_holder++;
    }

    // allocate size in memory for our array. Size of a character is 1 byte * count
    *ptr_main_array = (char**)malloc(sizeof(char*) * count);
    if (*ptr_main_array == NULL) {
        exit(1);
    }

    ptrTemp_string_holder = ptr_original_string;

    // Now start iterating through the whole unsplit string as long as we're not at the end
    while (*ptrTemp_string_holder != '\0')
    {
        // If the pointer points to a delimiter, i.e a comma, that means we are starting to read a new string
        if (*ptrTemp_string_holder == delimiter)
        {
            // Now allocate a memory size for a pointer to a pointer of the new string to be built
            (*ptr_main_array)[i] = (char*)malloc(sizeof(char) * split_string_len);

            // If its null, like some GPRMC or GPHDT results that come back empty, just exit and return back to main
            if ((*ptr_main_array)[i] == NULL) 
            {
                exit(1);
            }

            // Reset the token length and just move the hell on
            split_string_len = 0;
            i++;
        }
        ptrTemp_string_holder++;
        split_string_len++;
    }

    // If we are not at a delimiter however, we just allocate a size based on our token length to a pointer of a pointer
    // Or if you want, call it a pointer to an array
    (*ptr_main_array)[i] = (char*)malloc(sizeof(char) * split_string_len);


    // If for some unknown reason it was null, just stop the crap and return back to main...after all we got a shitty GPS device
    if ((*ptr_main_array)[i] == NULL) exit(1);

    i = 0;
    ptrTemp_string_holder = ptr_original_string;
    t = ((*ptr_main_array)[i]);

    // Now that we got what we need, we rebuild back everything to formulate a pointer to a pointer of character strings
    // I think then the rest is straight forward
    while (*ptrTemp_string_holder != '\0')
    {
        if (*ptrTemp_string_holder != delimiter && *ptrTemp_string_holder != '\0')
        {
            *t = *ptrTemp_string_holder;
            t++;
        }
        else
        {
            *t = '\0';
            i++;
            t = ((*ptr_main_array)[i]);
        }
        ptrTemp_string_holder++;
    }

    // Free the space that was allocated to this pointer
    free(ptr_main_array);

    // We return back the number of times we need to iterate to get the split components of the original string
    return count;
}

Answer 1

You have two invalid free in your code

One here :

char *ptrTemp_string_holder;

// Points to the first character of a split string from the main string 
char *t;

   free(ptrTemp_string_holder );

while ptrTemp_string_holder is not yet initialized

The second just before the end :

// Free the space that was allocated to this pointer
free(ptr_main_array);

because you try to free the local variable arr in the calling function

These two free must be removed.

Note the definition of ptrTemp_string_holder must be

const char *ptrTemp_string_holder;

because it receives the value of ptr_original_string

---

Its going to run out of memory because of failure to deallocate the memory assigned

You need to free *ptr_main_array and the memorized array, it seems strange to do that in split else split do not return a usable result, that must be done in the caller function, for instance adding the following main :

int main()
{
  char **arr = NULL;
  int count = split("aze,qsd", ',', &arr);

  if (arr != NULL) {
    for (int i = 0; i != count; ++i)
      free(arr[i]);

    free(arr);
  }
}

---

Compilation and execution under valgrind :

pi@raspberrypi:/tmp $ gcc -pedantic -Wall -Wextra -g s.c
pi@raspberrypi:/tmp $ valgrind --leak-check=full ./a.out
==10755== Memcheck, a memory error detector
==10755== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==10755== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==10755== Command: ./a.out
==10755== 
==10755== 
==10755== HEAP SUMMARY:
==10755==     in use at exit: 0 bytes in 0 blocks
==10755==   total heap usage: 3 allocs, 3 frees, 16 bytes allocated
==10755== 
==10755== All heap blocks were freed -- no leaks are possible
==10755== 
==10755== For counts of detected and suppressed errors, rerun with: -v
==10755== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 6 from 3)

Answer 2

This makes completely no sense for me:

    // str_gprmc is a string thats trying to be split 
    // "$GPRMC,130133.00,A,5741.6029848,N,01158.3855831,E,11.522,170.0,270319"

    for (int k = 0; k < ARRAY_SIZE(str_gprmc); k++)
    {
        char **arr = NULL;

        // Now do the splitting please
        split(str_gprmc[k], ',', &arr);
    }´

    // and the split function
int split(const char *ptr_original_string, char delimiter, char ***ptr_main_array)

If I get it correctly, str_gprmc is a variable of type char [] with some size, unspecified here. You iterate with the for(int k=....) loop across the array. The expression str_gprmc[k] extracts the k-th character from the array and passes it as a first parameter to the split() function, which expects a char pointer as a first parameter.

That means the numeric representation of the char-type data is interpreted as a numeric representation of the pointer (const char *). As a result you effectively run your split() on data at random addresses in initial 120 bytes of memory or so, dictated by numeric values (ASCII codes) of characters in your NMEA message – but certainly you do not proces the message itself.