Why does this not concatenate two strings?

Question

This is the requirement for my code:

This function appends the src string to the dest string, overwriting the terminating null byte ('\0') at the end of dest, and then adds a terminating null byte.

Returns a pointer to the resulting string dest.

This is the output I am getting:

Hello
World!
Hello World!
Hello

Here is my code:

char *_strcat(char *dest, char *src) {
    int lengthd = 0;
    int lengths = 0;
    int i = 0;
    int j = 0;
    int k = 0;
    char tmp[10];
    while (dest[i] != '\0') {
        lengthd++;
        i++;
    }
    while (src[k] != '\0') {
        tmp[lengths] = src[k];
        lengths++;
        k++;
    }
    for (; j < lengths - 1; j++) {
        dest[lengthd + 1] = tmp[j];
    }
    dest[lengthd + 1] = '\0';
    return (dest);
}

int main(void) {
    char s1[98] = "Hello ";
    char s2[] = "World!\n";
    char *ptr;
    printf("%s\\n", s1);
    printf("%s", s2);
    ptr = _strcat(s1, s2);
    printf("%s", s1);
    printf("%s", s2);
    printf("%s", ptr);
    return (0);
}

Answer 1

Your code fails for multiple reasons:

• you use a temporary array to make a copy of the source string: this array tmp has a fixed length of 10 bytes, which is too small if the source string is longer than 10 bytes. Otherwise you will have undefined behavior when you write beyond the end of this array.
• there is really no need for this temporary array anyway.
• the final loop stops at lengths - 1, hence you stop before the last byte of the copy.
• you copy all bytes to the same position dest[lengthd + 1].
• you finally set the null terminator at the same position again.
• you never changed the null terminator at dest[lengthd] so the function appears to have no effect on dest.
• the tests in main() cannot produce the output you posted, probably because of a typo in "%s\\n".
• avoid using identifiers starting with an _.

Here is a modified version:

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

char *my_strcat(char *dest, char *src) {
    int i = 0;
    int k = 0;

    /* find the offset of the null terminator in dest */
    while (dest[i] != '\0') {
        i++;
    }
    /* copy the bytes from the src string there */
    while (src[k] != '\0') {
        dest[i] = src[k];
        i++;
        k++;
    }
    /* set the null terminator */
    dest[i] = '\0';
    /* return the pointer to the destination array */
    return dest;
}

int main(void) {
    char s1[98] = "Hello ";
    char s2[] = "World!";
    char *ptr;
    printf("%s\n", s1);
    printf("%s", s2);
    ptr = my_strcat(s1, s2);
    printf("%s", s1);
    printf("%s", s2);
    printf("%s", ptr);
    return 0;
}

Note that the source string is not modified and the offsets should have type size_t and can be incremented as a side effect of the assignment:

char *my_strcat(char *dest, const char *src) {
    size_t i = 0;
    size_t k = 0;

    /* find the offset of the null terminator in dest */
    while (dest[i] != '\0') {
        i++;
    }
    /* copy the bytes from the src string there */
    while (src[k] != '\0') {
        dest[i++] = src[k++];
    }
    /* set the null terminator */
    dest[i] = '\0';
    /* return the pointer to the destination array */
    return dest;
}

You can also use pointers instead of offsets:

char *my_strcat(char *dest, const char *src) {
    /* use a working pointer to preserve dest for the return value */
    char *p = dest;

    /* find the offset of the null terminator in dest */
    while (*p != '\0') {
        p++;
    }
    /* copy the bytes from the src string there */
    while (*src != '\0') {
        *p++ = *src++;
    }
    /* set the null terminator */
    *p = '\0';
    /* return the pointer to the destination array */
    return dest;
}

One final change: you can combine reading the source byte, copying to the destination and testing for the null terminator, which will have been copied already:

char *my_strcat(char *dest, const char *src) {
    /* use a working pointer to preserve dest for the return value */
    char *p = dest;

    /* find the offset of the null terminator in dest */
    while (*p != '\0') {
        p++;
    }
    /* copy the bytes from the src string there */
    while ((p++ = *src++) != '\0') {
        /* nothing */
    }
    /* the null terminator was copied from the source string */
    /* return the pointer to the destination array */
    return dest;
}