Problems with implementation of concatenation function in C

Question

I am still new to pointers in C and have been trying to make string functions. This one is supposed to concatenate two strings, but when I try to run it, it crashes.

void concatenate (char *a, char *b, int size_a, int size_b) {
    char *c, *d, *e;
    c = a;
    d = b;
    int i, j, k;
    int l = 1;

    for(i = 0; i<size_a; i++) {
        *(e+i) = *(c+i);
    }
    for (j = size_a; j<size_a+size_b; j++) {
        *(e+j) = *(d+(j-(j-l)));
        l++;
    }
    for (k = 0; k< size_a+size_b; k++){
        printf("%c",*(e+k));
    }
}

What am I doing wrong?

Crashes your compiler? Really? Can you show the error message? — Carl Norum
– Carl Norum, Commented Feb 19, 2013 at 19:21
Not only that your function takes char *a and char *b as parameters (meaningless names), but its body even starts with char *c, *d, *e; c = a; d = b; — LihO
– LihO, Commented Feb 19, 2013 at 19:21
Well I would like e to point to the new string. How would I go about doing this? — apkim221
– apkim221, Commented Feb 19, 2013 at 19:24

LihO · Accepted Answer · 2013-02-19 19:31:51Z

2

You are trying to access the memory where uninitialized pointer e points to, which leads to undefined behavior.

Your function should look like this:

char* concatenate (const char *a, const char *b, int size_a, int size_b) {
    // create new string:
    char* newStr = malloc (size_a + size_b + 1);
    int i, j;

    // copy first string into it:
    for (i = 0; i<size_a; i++)
        newStr[i] = a[i];

    // copy second string into it:
    for (j = 0; j < size_b; j++)
        newStr[i + j] = b[j];

    newStr[i + j] = '\0';
    return newStr;
}

Note that function doesn't change strings that are passed to it, thus they can be passed as const. The size_a and size_b are expected to be equal to strlen(a) and strlen(b), which also means that in case you always pass null-terminated strings into this function, you can calculate their length on your own within its body (so you can get rid of last 2 arguments).

edited Feb 19, 2013 at 19:31

answered Feb 19, 2013 at 19:24

LihO

42.3k11 gold badges103 silver badges169 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

apkim221 Over a year ago

How do I get it so "concatenate" returns a new string?

Tall Software Dude Over a year ago

Shouldn't the count of size_a and size_b include the null terminator. Then the malloc size parameter should be set to (size_a + size_b - 1).

LihO Over a year ago

@TallSoftwareDude: That's explained in the lines under the code.

LihO Over a year ago

@user2041197: You're welcome :) Also don't forget to call free on the pointer returned from concatenate function. You should free the memory that has been allocated by malloc once you don't need this memory anymore.

antlersoft · Accepted Answer · 2013-02-19 19:23:24Z

0

e is an uninitialized pointer, which means it is pointing to some random place in memory. When you start writing to it as if it were a pointer to a character array, your program will crash (or worse, change some state mysteriously causing unspecified behavior later on).

answered Feb 19, 2013 at 19:23

antlersoft

14.8k4 gold badges40 silver badges56 bronze badges

Comments

Theodoros Chatzigiannakis · Accepted Answer · 2013-02-19 19:30:44Z

0

Your code seems obfuscated for no reason. The problem is that you're assigning to e, which is uninitialized. The following is, pretty much, your code in a more readable form and with e allocated. I haven't tested it, though.

void concatenate (char *c, char *d) {    
    char *e; 
    e = calloc(strlen(c) + strlen(d) + 1, sizeof(char));
    int i, j;
    int l = 1;

    for(i = 0; c[i]; i++) {
        e[i] = c[i];
    }
    l = i;
    for (j = 0; d[j]; j++) {
        e[l + j] = d[j];
    }
    e[l + j] = '\0';

    printf("%s", e);

    free(e);
}

answered Feb 19, 2013 at 19:30

Theodoros Chatzigiannakis

29.5k9 gold badges72 silver badges108 bronze badges

1 Comment

chue x Over a year ago

the original passes in the string sizes. it is the norm, but how do you know that the strings are null terminated?

Collectives™ on Stack Overflow

Problems with implementation of concatenation function in C

3 Answers 3

4 Comments

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related