After returning from function when I printed modified string, It shows me the garbage value

Question

int main()
{

    //CODE
    printf("Enter the destination string : ");

    char *dest_string = (char *)malloc(sizeof(char));
    int le = 0;

    while(dest_string[le - 1] != '\n')
    {
        dest_string = (char *)realloc(dest_string,sizeof(char) * (le + 1) );
        dest_string[le] = getchar();
        le++;
    }

    *(dest_string + le - 1) = '\0';
    le = 0;


    printf("Enter the source string : ");
    char *source_string = (char *)malloc(sizeof(char));
    le = 0;

    while(source_string[le - 1] != '\n')
    {
        source_string = (char *)realloc(source_string,sizeof(char) *  ( le + 1 ) );

        source_string[le] = getchar();

        le++;
    }

    *(source_string + le - 1) = '\0';

    _concatenate(dest_string, source_string);

    puts(dest_string);

    free(dest_string);
    free(source_string);

    exit(0);
}

/* following function takes two parameter, first parameter is
   destination string and second parameter is source string
   This function makes changes to destination string by 
   appending the source string to it */
void _concatenate(char *dest_string, char *source_string){


    int le = 0; // loop enumerator
    int dest_string_len = strlen(dest_string);

    while(source_string[le] != '\0')
    {

        dest_string = (char *)realloc(dest_string,sizeof(char) * (dest_string_len + le + 1) );

        *(dest_string + dest_string_len + le) = *(source_string + le);
        le++;
    }

    dest_string = (char *)realloc(dest_string,sizeof(char) * (dest_string_len + le) );
    *(dest_string + dest_string_len + le  ) = '\0';

    puts(dest_string);

    return ;
}

Answer 1

Many issues to note here, both major and minor, in no particular order:

1. The two standard signatures for main are: int main(int argc, char **argv) and int main(void).

2. Identifiers that start with an _ should be avoided as they are, with very few exceptions, reserved for the implementation.

3. exit(0) at the end of main is largely unnecessary, as main will implicitly return 0; if it reaches its terminating } (since C99).

4. sizeof (char) is guaranteed to be 1.

5. You do not need to cast the return of malloc, as a void * can be safely and implicitly converted to any other pointer type. Casting the return is considered a bad practice.

6. As pointed out in the comments if (dest_string[le - 1] != '\n') will access dest_string[-1] on the first iteration, which is out of bounds. This is undefined behavior. Of course this holds true for the loop involving source_string as well.

7. You need to check if *alloc functions fail, by testing their return value is not NULL. You cannot blindly reassign the return of realloc, because in the case where it fails, the original pointer will still be valid, and will you need to free the original pointer sooner or later.

8. On the other hand, if realloc succeeds, the previous pointer value should be considered invalid. While it's true that the base address of the memory block may not have changed, you cannot know this without checking first, and it is rather pointless to test if they are the same. If the return is not NULL, simply use that pointer.

9. You need to test if getchar returns the end-of-file marker (EOF), otherwise, if it ever occurs, you're going to loop until you run out of memory.

You haven't shown us any headers, so I have to assume you didn't include them. You'll need

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

Whenever you repeat yourself, it's almost always a sign that you should recompose those sections into some form of abstraction. In this example, the identical sections of code you've written to build strings from stdin should be reduced into a function.

Since you do not error check realloc, we have to assume it always succeeds. As such, #8 on our list shows us that dest_string in main cannot reliably be assumed to point to valid memory after the call to _concatenate, which may have moved the data located at that pointer, and freed the old data

If there is not enough room to enlarge the memory allocation pointed to by ptr, realloc() creates a new allocation, copies as much of the old data pointed to by ptr as will fit to the new allocation, frees the old allocation, and returns a pointer to the allocated memory.

and so puts(dest_string) might access invalid or uninitialized memory, and free(dest_string) might cause a double free.

You use strlen to get the length of your destination string, but then you manually count and copy the source string. Using other standard library functions makes this much cleaner:

char *concat(char *dest, char *src) {
    size_t dest_len = strlen(dest),
           src_len = strlen(src);

    char *p = realloc(dest, dest_len + src_len + 1);

    return p ? strcat(p, src) : NULL;
}

As you can see, managing the memory and doing the actual string operation should really be separate concerns; strcat doesn't care if the destination buffer is dynamically allocated or not, as long as the caller makes sure there's enough space for the result.

Reallocating for every byte is not a great strategy, but it is simple, and does work.

---

Here's a complete example program:

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

char *concat(char *, char *);
char *input(const char *);

int main(void) {
    char *dest = input("Enter the destination string : "),
         *source = input("Enter the source string : "),
         *result;

    if (!dest || !source || !(result = concat(dest, source))) {
        free(dest);
        free(source);

        fprintf(stderr, "Failed to allocate memory.\n");
        return EXIT_FAILURE;
    }

    puts(result);

    free(result);
    free(source);
}

char *concat(char *dest, char *src) {
    size_t dest_len = strlen(dest),
           src_len = strlen(src);

    char *p = realloc(dest, dest_len + src_len + 1);

    return p ? strcat(p, src) : NULL;
}

char *input(const char *msg) {
    int ch;
    size_t length = 0;
    char *buf = malloc(1),
         *rebuf;

    if (!buf) return NULL;
    if (msg) printf("%s", msg);

    while ((ch = getchar()) != EOF && ch != '\n') {
        if (!(rebuf = realloc(buf, length + 1))) {
            free(buf);
            return NULL;
        }

        (buf = rebuf)[length++] = ch;
    }

    buf[length] = '\0';

    return buf;
}