unable to return the updated string from called function and free the memory

Question

I have a simple program with two string which I supposed to concatenate in the called function and return to the calling function.

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


void call_func(char *str1,char *str2)
{

  char *msg_concat;
  asprintf(&msg_concat,"%s%s", str1, str2);
  printf("%s",msg_concat);
  str2 = msg_concat;

}



int main()
{
    char *str1="new" ;

    char *full_msg ="test";
    call_func(str1,full_msg);
    printf("value of full_msg=%s",full_msg);
    return 0;

}

I have used asprintf() for concatenation but the value is not returned in the main function. As I have used the pointer, is not it is expected to return the changed value??

Further while assigning the pointer , in my understanding reference is copied rather than the values. now here I need to free the allocated memory msg_concat otherwise there will be memory leak . how am supposed to free memory if the function uses pointer and return the reference to the calling function??

free(msg_concat) is not working while in the last line of called function as-

void call_func(char *str1,char *str2)
{

  char *msg_concat;
  asprintf(&msg_concat,"%s%s", str1, str2);
  printf("%s",msg_concat);
  str2 = msg_concat;
   free(msg_concat);

}

actually in my recent project, I have the scenerio that the calling function has

GSList *parsed_msg = LineParser(dev_ip,encoded_msg, "\n", file_path, unique_id);

and the called function is

GSList *LineParser(char* dev_ip, char *msg_full, char *separator, char *file_path, int unique_id)
{
    GSList *parsed_msg = NULL;
    char connection_id[50];
    sprintf(connection_id,"%s|%d", dev_ip, unique_id);
    char *msg_concat;


    // inserting {file_path : last_not_complete_line} logic to TCP_CACHE
    //and removing the correspoing last_line
    g_mutex_lock (&mutex_hash_main);

    // char *last_line = (char *) (g_hash_table_lookup((GHashTable *) g_hash_table_lookup(TCP_CACHE, connection_id), file_path));

    GHashTable *t_filepath_msg_dict = NULL;         //for {file_path : last_not_complete_line}
    if (TCP_CACHE != NULL)
    {
        t_filepath_msg_dict = (GHashTable *)(g_hash_table_lookup(TCP_CACHE, connection_id));
        if (t_filepath_msg_dict != NULL)
        {
            char *last_line = (char *) (g_hash_table_lookup(t_filepath_msg_dict, file_path));

            if(last_line != NULL)   //if the hash has device ip, append the value to msg
            {
                zlog_debug(c,"concatenating: str1: %s and str2: %s\n", last_line, msg_full);
                asprintf(&msg_concat,"%s%s", last_line, msg_full);


                //msg_concat = concat(last_line,msg_full);
                g_hash_table_remove(t_filepath_msg_dict, file_path);

                msg_full = msg_concat;

            }   
        }

    }


    int msg_len = strlen(msg_full);
    char last_char = msg_full[msg_len - 1];
    zlog_debug(c, "len of message: %d", msg_len);
    zlog_debug(c, "last char is : %c", last_char);
    char *token=NULL;
    char *remaining_str=NULL;
    token = strtok_r(msg_full, "\n", &remaining_str);

    while(token != NULL)
    {
        if(token[0]==' ')
        {
            token = trimwhitespace_parser (token);
            if(strcmp(token,"")==0)
            {
                //insert this token to GSList
                parsed_msg = g_slist_prepend (parsed_msg, token);
                token = strtok_r(NULL, "\n", &remaining_str);
                continue;
            }
        }
        if(strcmp(remaining_str,"")==0)
        {
            if(strlen(token) > 10000)
            {
                zlog_warn(c, "Message too big(more than 10000 len). Stop looking for new line and process msg");
                g_hash_table_remove(t_filepath_msg_dict, file_path);
            }
            else
            {
                if(last_char=='\n')
                {
                    //new line is the last character. do nothing
                    zlog_debug(c, "last character is new line");
                }
                else
                {
                    zlog_debug(c, "last character is not new line");
                    //new line not received
                    if (t_filepath_msg_dict == NULL)        //insert new record
                    {
                        GHashTable *each_filepath_msg_dict = g_hash_table_new_full(g_str_hash, g_str_equal, key_str_destroy_cb_parser, value_str_destroy_cb_parser);
                        zlog_debug(c,"Inserting file_path: %s to connection_id: %s", file_path, connection_id);

                        g_hash_table_insert(each_filepath_msg_dict, strdup(file_path), strdup(token));
                        g_hash_table_insert(TCP_CACHE, strdup(connection_id), each_filepath_msg_dict);
                    }
                    else        //update existing record
                    {
                        zlog_debug(c,"Connection_id :%s is already found; appending/replacing file_path :%s", connection_id, file_path);
                        g_hash_table_insert(t_filepath_msg_dict, strdup(file_path), strdup(token));
                    }
                    g_mutex_unlock(&mutex_hash_main);
                    return parsed_msg;
                }
            }
        }
        //insert token to GSList
        parsed_msg = g_slist_prepend (parsed_msg, token);
        token = strtok_r(NULL, "\n", &remaining_str);
    }
    g_mutex_unlock(&mutex_hash_main);
    return parsed_msg;
}

I have seen that i have problem with memory leak as i have to free msg_concat,as in the given answer it is said that the changed value doesnot return to calling function. where is the appropriate place to free the asprintf msg_concat pointer???

Answer 1

As C uses pass-by-value for function parameter passing, in your code

str2 = msg_concat;

is pretty much useless, in regard to returning the value to main(). Instead, you may want to use strcpy() to copy the content to the memory area pointed by str2, which will reflect in main().

Also, after you have copied the content, you can free() the msg_concat before the end of call_func().

---

Now, even this won't solve your issue, as you're calling call_func() with a second argument as a string literal. This is illegal in two ways

• modification of string literal will cause UB.
• the destination string (as used in strcpy()) won't have enough memory to hold the final output.

Solution: A possible way to solve this can be

Change the type of full_msg to a fixed length array, which has enough length to hold the concatenated output, like

char full_msg[32] = "test";

then, with a call like

call_func(str1,full_msg);

and inside, with a strcpy(str2, msg_concat);, you can achieve your goal.

Additionally, you can use free(msg_concat); also.

Answer 2

Pointers are passed by value, so the last assignment has no effect in the caller. There are two ways to do what you wish:

• Pass a pointer to pointer for str2, or
• Return a new string to the caller.

Returning the result has a slightly more explicit appearance to the callers of your function, but the pointer-to-pointer approach is valid as well.

An additional issue that you need to solve is freeing the pointer passed into your function. You cannot pass a string literal if you would like to free the value:

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

char* call_func(char *str1,char *str2) {
    char *msg_concat = NULL;
    asprintf(&msg_concat,"%s%s", str1, str2);
    printf("%s",msg_concat);
    free(str2);
    return msg_concat;
}

int main() {
    char *str1="new" ;
    char *full_msg =strdup("test"); // Put the string in dynamic memory
    full_msg = call_func(str1, full_msg);
    printf("value of full_msg=%s", full_msg);
    free(full_msg);
    return 0;
}