What is wrong with my 'append' algorithm or code?

Question

2 strings are given, second word will be append to first one and 3rd variable will store this. For example;

char *str1 = "abc";
char *str2 = "def";
char *str3 = "abcdef"; //should be

Here is my code, I get runtime error:

#include <stdio.h>
#include <malloc.h>

void append(char *str1, char *str2, char *str3, int size1, int size2)
{
    int i=0;
    str3 = (char*) malloc(size1+size2+1);
    str3 = str1;

    while (str2[i] != '\0') {
        str3[i+size1] = str2[i];
        i++;
    }

    str3[size1+size2] = '\0';
}

int main() 
{
    char *str1 = "abc";
    char *str2 = "def";
    char *str3;

    append(str1, str2, str3, 3, 3);

    return 0;
}

still doesn't work, looks like this is not the main problem. — sedooe, May 29 '14 at 00:31
What specifically fails with your code? What line causes the runtime error? What runtime error do you get? Please [edit] your question to be more specific about the problem you're having. — Ken White, May 29 '14 at 00:31
There no specific fail man, no wrong about any line. Just runtime error, 'windows has stopped working'. That's all. — sedooe, May 29 '14 at 00:36

score 2 · Answer 1 · edited May 23 '17 at 11:57

2

str3 = (char*) malloc(size1+size2+1);
str3 = str1;

Here's your problem. Doing this replaces the pointer to the correct amount of space from malloc to the pointer where str1 is contained. Keeping with your loop design, change this to:

str3 = malloc(size1+size2+1); 
for (int j = 0; str1[j] != '\0'; j++)
    str3[j] = str1[j];

Also, see this question/answer about casting the result of malloc in C: Do I cast the result of malloc?

edited May 23 '17 at 11:57

Community

1
1

answered May 29 '14 at 00:32

IllusiveBrian

3,105
2
14
17

@lombranzo Let's say `str1` is in memory space `1` and `malloc(size1+size2+1)` returns a pointer to memory space `2`. First `str3` is assigned `2`, but then you immediately assign it to the value of `str1`, which is `1`. `1` only has `size1 + 1` spaces allocated, so when you try to read `str3[2 + size1]` you are reading past the end of your allocated space for that pointer, which is undefined behavior. – IllusiveBrian May 29 '14 at 01:00

score 0 · Answer 2 · answered May 29 '14 at 01:37

There is another issue with the code. You pass pointer by value. So any malloc inside a function will do only local changes. After function ends your pointer will still point to the old value. You should pass a pointer to pointer if you want to change it. See an example:

#include <stdio.h>
char *c = "Second";
void assign(char *s) { s = c; }

int main() 
{
    char *str = "First";
    assign(str);
    printf("String after assign: %s\n", str);
    return 0;
}

After running the program you will see 'First' in you console. The correct code is:

#include <stdio.h>
char *c = "Second";
void assign(char **s) { *s = c; }

int main() 
{
    char *str = "First";
    assign(&str);
    printf("String after assign: %s\n", str);
    return 0;
}

score 0 · Answer 3 · answered May 29 '14 at 14:15

#include <stdio.h>
#include <stdlib.h> //to standard
#include <string.h>

char *append(const char *str1, const char *str2, int size1, int size2){
//parameter char *str3 is local variable.
//It is not possible to change the pointer of the original.
//str3 = str1;//<<-- memory leak
//str3[i+size1] = str2[i];//<<-- write to after str1(can't write!)

    char *str3 = (char*) malloc(size1+size2+1);
    memcpy(str3, str1, size1);//copy to alloc'd memory.
    memcpy(str3 + size1, str2, size2);//copy to after str1
    str3[size1+size2] = '\0';
    return str3;
}

int main(){
    char *str1 = "abc";
    char *str2 = "def";
    char *str3;

    str3 = append(str1, str2, 3, 3);
    printf("%s\n", str3);

    return 0;
}

What is wrong with my 'append' algorithm or code?

3 Answers3