realloc() is not resizing the array of pointers-CodePudding

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?

CodePudding user response：

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;
}

CodePudding user response：

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?