2

Here I tried implementing strcat function in C. Problem is that when I am printing the source string it is showing me "hyam" not "shyam"

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

char *fun(char *target, const char *source) {
    printf("%s\n", source);
    int x = strlen(target);
    while (*source != '\0') {
        *(target + x++) = *source++;
    }
    *(target + x) = '\0';
    return target;
}

int main() {
    char target[] = "ram";
    char source[] = "shyam";
    fun(target, source);
    printf("%s\n", target);
    printf("%s", source);
    return 0;
}

Here in last line of output hyam is shown but it should be shyam.

shyam
ramshyam
hyam
Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39
Ankit Bhardwaj
  • 77
  • 5
  • 5
    Because you can't push into the array more than it's size. – Eugene Sh. Oct 19 '21 at 14:41
  • 1
    in c++ you should use `std::string` – apple apple Oct 19 '21 at 14:41
  • 1
    `target` is an array with four elements, and it needs at least nine. Reading or writing outside an array has undefined behaviour. – molbdnilo Oct 19 '21 at 14:42
  • 1
    It's not necessarily the problem but `int x=strlen(target)` should be `size_t x`. – sj95126 Oct 19 '21 at 14:42
  • 3
    If question does not address C++ explicitly please don't add that tag! This looks like a pure C question. However, in C `main` should be declared as `int main(void)`, if you don't need command line arguments. – Aconcagua Oct 19 '21 at 14:43
  • Side note: There's syntactic sugar for `*(array + index)`: `array[index]`... – Aconcagua Oct 19 '21 at 14:45
  • 1
    You don't need that offset at all if you simply do: `target += strlen(target);` – then you can have `*target++ = *source++` inside the loop. – Aconcagua Oct 19 '21 at 14:46
  • `char target[]="ram"` is equivalent to `char target[4]="ram"`. There is only space for 3 characters (+ the terminating null). You want e.g. `char target[100]="ram";`, then there will be space for 99 characters. – Jabberwocky Oct 19 '21 at 14:47
  • C or C++ tag please pick one! – Wyck Oct 19 '21 at 14:51
  • You should do `do { *target++ = *source++; } while(*source);` In this way, testing for NUL character at the end, you also copy the final NUL – alinsoar Oct 19 '21 at 14:56

2 Answers2

6

Your target array is too small - it needs at least nine elements (the length of both strings plus a terminating zero).

Writing outside target has undefined behaviour, but in practice, it looks like your arrays happen to be laid out end to end, like this:

 |r|a|m|\0|s|h|y|a|m|\0|
 ^        ^
 |        | 
target   source

and then you concatenate, going past the end of target into source:

 |r|a|m|s|h|y|a|m|\0|\0|
 ^       ^
 |       | 
target  source

which makes it look like an 's' has disappeared.

(Note that this is undefined, so anything can happen. You can't rely on this behaviour, unless the documentation for your compiler says that it's fine and should do this. Which it most likely won't.)

molbdnilo
  • 64,751
  • 3
  • 43
  • 82
2

The problem is the array target defined with char target[] = "ram"; is too short to accommodate appending the contents of the source string. You must make target larger, at least 9 bytes, 8 for the characters and one more for the null terminator:

char target[9] = "ram";
chqrlie
  • 131,814
  • 10
  • 121
  • 189