0

So I have to make a function to concatenate two strings in C; The function creates a new string by concatenating str1 and str2. The function has to call malloc() or calloc() to allocate memory for the new string.The function returns the new string.

After executing the following call to printf() in a main test function: printf ( “%s\n”, myStrcat( “Hello”, “world!” )); the printout on the screen has to be Helloworld!

Here's my code so far; I can't quite understand why it does not work. It doesn't do anything... it compiles and runs but nothing is displayed.

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

char *my_strcat( const char * const str1, const char * const str2);

int main()
{
    printf("%s", my_strcat("Hello", "World")); // test function. Output of print statement    is supposed to be HelloWorld
}

char *my_strcat( const char * const str1, const char * const str2)
{

    char *temp1 = str1; // initializing a pointer to the first string
    char *temp2 = str2; // initializing a pointer to the second string

    // dynamically allocating memory for concatenated string = length of string 1 + length of string 2 + 1 for null indicator thing.
    char *final_string = (char*)malloc (strlen(str1) + strlen(str2) + 1);

    while (*temp1 != '\0') //while loop to loop through first string. goes as long as temp1  does not hit the end of the string
    {
        *final_string = *temp1; // sets each successive element of final string to equal each successive element of temp1
        temp1++; // increments address of temp1 so it can feed a new element at a new address
        final_string++; // increments address of final string so it can accept a new element at a new address
    }
    while (*temp2 != '\0') // same as above, except for string 2.
    {
        *final_string = *temp2;
        temp2++;
        final_string++;
    }

    *final_string = '\0'; // adds the null terminator thing to signify a string
    return final_string; //returns the final string.
}
Improve this question
3
  • 3
    Don't cast malloc in C. Commented Oct 23, 2014 at 14:47
  • 1
    Please indent your code properly when posting here. Commented Oct 23, 2014 at 14:48
  • 2
    *final_string = '\0'; return final_string; <--- Look at what you're returning, here. Commented Oct 23, 2014 at 14:49

5 Answers 5

Reset to default
4

You're returning final_string, but it's been incremented during the course of your algorithm to point to the null terminator - not the beginning of the string.

You need to change the allocation to something like:

char *final_string_return = malloc(strlen(str1) + strlen(str2) + 1);
char *final_string = final_string_return;

And then later on:

return final_string_return;
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

3

You should store in another variable the result of malloc (which you should always test against failure) :

char *result_string = malloc (strlen(str1) + strlen(str2) + 1); 
if (!result_string) { perror("malloc"); exit(EXIT_FAILURE); };
final_string = result_string;

and finally return it

return result_string;

So the name final_string is unfortunate; make it perhaps current_pointer !

BTW, your function is working only with the convention that the caller should free its result. You should document that convention (at least in comments). In particular in your main function

printf("%s", my_strcat("Hello", "World"));

is a memory leak (you never free the result of that my_strcat call, but you should).

Please take the habit of compiling with warnings and debug info (e.g. gcc -Wall -Wextra -g with GCC), and learn how to use the debugger (gdb), since you could step by step your program in the debugger. Use also valgrind to detect -by testing- some memory leaks.

Improve this answer

1 Comment

Thanks for the suggestions! I'll try to educate myself on some of those things; at this point, & with my knowledge level your last paragraph went several miles over my head.
0 
int main( void )
{
    char * concatStr = my_strcat("Hello", "World");

    printf("%s", concatStr); // test function. 
    // Output of print statement    is supposed to be HelloWorld
    free( concatStr ); // note this is safe, even if my_strcat() returns NULL
    return(0);
}

char *my_strcat( const char * const str1, const char * const str2)
{


    // dynamically allocating memory for concatenated string 
    // = length of string 1 + length of string 2 + 1 for null indicator thing.
    // and sets all bytes to '\0'
    char *final_string = calloc ((strlen(str1) + strlen(str2) + 1), 1);
    if( NULL == final_string )
    { // then calloc() failed
        return( NULL );
    }
    // implied else, calloc() successful

    strcpy( final_string, str1 );
    strcat( final_string, str2 );

    return( final_string ); // note: caller must invoke free() 
                            //       to avoid memory leak.
}
Improve this answer

Comments

0

You can always just use sprintf

char* my_strcat(const char* const s1, const char* const s2)
{
    char *dst = malloc(strlen(s1) + strlen(s2) + 1);
    if (dst == NULL)
    {
      return NULL;
    }
    sprintf(dst, "%s%s", s1, s2);
    return dst;
}
Improve this answer

Comments

-2

The final_string points to the end of destination string.

Following code returns the final string:

return (final_string-strlen(str1)-strlen(str2)-1); //returns the final string.
Improve this answer

2 Comments

address of final_string minus length of str1 minus length of str2 minus 1 will point to la-la land, somewhere on the stack
You mean that address of final_string would less than length of str1 add length of str2 add 1?I ignored this problem ...

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.