realloc() is not resizing the array of pointers

Question

The main idea of this program is to read the data and extract File and folder name from the buffer. The payload data can look like this : cached_folder\test1_file. The function get_folder_file_name extracts the folder and file name since we don't know the length of data so it's reasonable to use realloc() function to expand buffer as per requirement since it's running on memory limited device (embedded).

Here is the code :

typedef struct command_packet
{
    uint32_t trigger_mode;
    uint32_t command_status;
    uint8_t payload_data[];
}COMMAND_PACKET;

//This function is called in main whenever there is incoming packet from TCP which has payload, command status and mode.
int32_t Analyse_stream(COMMAND_PACKET *pcommand, int32_t payload_data_length)
{
    char *file_name = (char *)malloc(sizeof(char));
    char *folder_name = (char *)malloc(sizeof(char));
    int file_name_len= 0;
    int folder_len = 0;
    uint16_t status;

    
    if ((status = get_folder_file_name(pcommand->payload_data, payload_data_length, file_name,folder_name, &file_name_len, &folder_len )) != SUCCESS)
    {
        free(file_name);
        free(folder_name);
        return status;
    }

   printf("File Name = %s\n", file_name);
   printf("Folder = %s\n", folder_name);

   free(file_name);
   file_name = NULL;

   free(folder_name);
   folder_name = NULL;
}

//This function is called in stored_byte_stream to extract the folder and file name.
int32_t get_folder_file_name(uint8_t *payload_data, int32_t payload_data_len, char *file_name, char *folder_name, int32_t *file_name_len, int32_t *folder_len )
{
    int i = 0;
    int k=0;
    // Check until null character to get the file name
    // For alignment with u16, single null char is added at the end of the file name for
    // a file name with strlen as odd and two null chars for a file name with even strlen
    do{
        if(payload_data[i] == '/')
        {
          if(i > 0)
              k = i+1;
        }
        i++;
    }while((*(payload_data + i) != '\0') && (i < payload_data_len));

    if(k == 0)
    {
        return FOLDER_NAME_IS_EMPTY;
    }

    if (i == payload_data_len)
    {
        // Null char search completed till the end of the payload len, No filename found
        return FILE_NAME_NOT_FOUND;
    }

    if (i == 1)
    {
        // File Name strlen = 0; implies that the file name was not provided
        return FILE_NAME_IS_EMPTY;
    }

    *folder_len = k+1;
    *file_name_len= i-k+1;
    
    folder_name = realloc(folder_name, sizeof(char) *(*folder_len));
    strncpy(folder_name, (char *)payload_data, *folder_len -1);
    

    file_name = realloc(file_name, sizeof(char) *(*file_name_len));
    strncpy(file_name, (char *)payload_data + k, *file_name_len);

    return SUCCESS;
 }

So, the issue I am facing is whenever the size of file_name_len or folder_len exceeds by 13, it prints nothing. The function get_folder_file_name returns empty file name or folder name whichever exceeds by 13. But in this same function folder_name & file name also has correct data but when they return if any of them exceeds by 13 length, buffer is empty. The issue gets fixed if I allocate fixed memory in initialization like char *file_name = (char *)malloc(15); and char *folder_name = (char *)malloc(15); and then doing realloc() it gets fixed. What I am doing wrong?

Have you tried to debug your program? For example just run it in a debugger to see if it crashes. And if there's no crash, step through the code statement by statement (while monitoring variables and their values) to see what really happens? — Some programmer dude
– Some programmer dude, Commented Mar 23, 2022 at 8:18
malloc(sizeof(char)) -- That's not much. (It's just one byte, just enough for the terminator on an empty string.) — M Oehm
– M Oehm, Commented Mar 23, 2022 at 8:19
Also, in C you don't have to (and really shouldn't) cast the result of malloc or realloc. I would say that you shouldn't cast anything by default, unless the compiler complains. — Some programmer dude
– Some programmer dude, Commented Mar 23, 2022 at 8:19
And sizeof(char) is specified to always be equal to 1 (so you don't have to multiply by that). And strncpy might not add a null-terminator in your string (and are you sure you allocate space for it). — Some programmer dude
– Some programmer dude, Commented Mar 23, 2022 at 8:21
If you let get_folder_file_name handle the allocation, you should alter the function so that the calling function knows about the changes you've made to the pointer. As is, you just re-allocate to a local variable, which will be lost after get_folder_file_name returns. You could return the handle to the allocated memory: char *p = get_folder_file_name(...), for example. — M Oehm
– M Oehm, Commented Mar 23, 2022 at 8:23

M Oehm · Accepted Answer · 2022-03-23 11:00:29Z

The problem in your code is not how you allocate the strings in get_folder_file_name, but that the changes you've made in that function are not visible in Analyse_stream. The file_name and file_folder in that function are still the 1-byte chunks that you allocated first.

(Perhaps you got segmentation violations when you tried to print these trings when you didn't allocate and you felt the need to allocate dummy strings first. That isn't necessary; all proper allocation is done in get_folder_file_name.)

It is very common for functions to allocate or re-allocate memory, so there are several idioms for doing so.

A very common one ist to return the allocated data:

char *get_file_name(..., uint32_t *length)
{
    char *result;

    // determine string s of length l somehow

    result = malloc(l + 1);
    if (result == NULL) return NULL;

    *length = l;
    strcpy(result, s);

    return result;
}

And use it like this:

uint32_t len;
char *fn = get_file_name(..., &len);

// use fn if not NULL

free(fn);

In your case, you have two strings, so that may be impractical. You could also pass in a pointer to each string, so that the function can modify the string through it the same way you already do for the lengths:

int get_ffn(..., char **file, char **folder)
{
    // determine strings s1 and s2 of lengths l1 and l2 somehow

    *file = malloc(l1 + 1);
    *folder = malloc(l2 + 1);

    if (*file == NULL || *folder == NULL) {
        free(*file);
        free(*folder);
        return MEMORY_ERROR;
    }

    strcpy(*file, s1);
    strcpy(*folder, s2);

    return OK;
}

and then call this in Analyse_stream:

char *file;
char *folder;
int error = get_ffn(..., &file, &folder);

If error == OK, the strings now have the relevant data, which you must free after using.

That's a lot of pointers to pass. You could make the interface simpler by creating a structure:

struct path {
    char *folder;
    char *file;
};

int get_path(struct path *path, ...)
{
     // ...

     path->file = malloc(l1 + 1);
     path->folder = malloc(l2 + 1);

     return OK;
}

and call it like this:

struct path path;

if (get_path(&path, ...) == OK) ...

But you must free the strings in the struct like in the other examples above after you're done.

If you know that the size of your payload is reasonably small and fits on the stack, you could do without dynamic allocation and create two strings that another function fills in:

char file[PAYLOAD_MAX];
char folder[PAYLOAD_MAX];

if (fill_in_ff(..., file, folder) == OK) ...

The function then just takes these char arrays, which will "decay" into a pointer, as arguments:

int fill_in_ff(..., char *file, char *folder)
{
    // ... find out lengths ...
    // ... no allocation ...

    memcpy(file, s1, l1);
    memcpy(file, s2, l2);

    s1[l1] = '\0';
    s2[l2] = '\0';

    return OK;
}

Got it. I was making passing by value not by reference. Issue is fixed. Since now I am using double pointer in the function. How do I iterate it, I need to make file name & folder name to upper case and remove white space. Earlier I was using for loop on single pointer as array to do the same but now after using double pointer it gives warnings
You can access the ith character of char **p via (*p)[i]. Alternatively, use a single pointer to iterate over the contents of your string as usual: for (char *q = *p; *q, q++) *q = tolower(*q);
Basically, yes, but you must use the (*file_name)[i] notation throughout, of course. Do yourself a favour and declare an auxiliary pointer char *p = *file:name. That makes reading the code much easier.
Okay. I tried running the code, it gets stuck after iterating once at line (*file_name)[counter++]=(*file_name)[i];

Lundin · Accepted Answer · 2022-03-23 10:53:34Z

The pointer returned by realloc is not necessarily still pointing at the same address as the one passed.

In Analyse_stream, the file_name and folder_name might no longer point at the relevant address once you have called realloc. They don't get updated, because in get_folder_file_name you only update local copies of those variables and not the ones one the caller side.

You essentially have a more complex version of this FAQ bug: Dynamic memory access only works inside function

Although this whole program looks needlessly slow and complicated - why can't you just allocate a fixed size like 256 bytes statically and skip all the slow reallocation calls?

Unrelated to your question, please note that strncpy is a dangerous function that should pretty much never be used for any purpose. See Is strcpy dangerous and what should be used instead?

Collectives™ on Stack Overflow

realloc() is not resizing the array of pointers

2 Answers 2

8 Comments

Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

8 Comments

Comments

Linked

Related