8

I tried to implement the strcat by myself, and I found the strcat implementation from Wiki like this......but when I use it, there is segmentation fault.

What's wrong with the code below?

char *
strcat(char *dest, const char *src)
{
    size_t i,j;
    for (i = 0; dest[i] != '\0'; i++)
        ;
    for (j = 0; src[j] != '\0'; j++)
        dest[i+j] = src[j];
    dest[i+j] = '\0';
    return dest;
}
bmargulies
  • 97,814
  • 39
  • 186
  • 310
skydoor
  • 25,218
  • 52
  • 147
  • 201
  • 3
    The code seems fine. Are you sure you use it correctly? What's the "client code"? – jpalecek Mar 21 '10 at 20:23
  • The question should be: "Why is Wikipedias strcat implementation wrong?". You should ask that on the wiki discussion page (http://en.wikipedia.org/wiki/Talk:Strcat). Besides, it's probably your client code (which may not even exist) that is broken. – Martin Wickman Mar 22 '10 at 07:59
  • Throw in a check for `dest` or `src` being null. – mpez0 Mar 21 '10 at 21:50
  • In Standard C, the library can do what it likes when either pointer is null; the behaviour is undefined. Legitimate undefined behaviour does include 'working sanely'; it also includes 'crashing with a core dump' and 'removing every file on the machine that you have permission to remove'. Don't risk it - don't call library functions with null pointers. – Jonathan Leffler Mar 21 '10 at 23:06
  • Quite true, but when re-implementing a library function, particularly if one is reporting segmentation violations, checking for nulls and following the principle of least surprise would seem the appropriate thing to do. – mpez0 Mar 22 '10 at 12:54
  • That's an awful idea. Most invalid pointers aren't null, so this doesn't help in practice. And when you do detect a null pointer, what are you going to do with it? Abort the program? Because that's what would happen anyway. – melpomene Aug 05 '17 at 13:25

5 Answers5

16

the code is okay.

Looks like you have a problem in the calling code.

Did you remember to allocate enough memory for the target string?

Pavel Radzivilovsky
  • 18,794
  • 5
  • 57
  • 67
10

I would strongly suggest using pointers rather than integer indexes, for fear of integer overflow. Even if size_t is the same number of bits as char *, you're adding indices where you wouldn't be adding pointers.

I guess that's more or less academical; if you're calling strcat() on multi-gigabyte strings you're probably in all sorts of trouble.

Here's a pointer-based version, for completeness' sake:

char *
my_strcat(char *dest, const char *src)
{
    char *rdest = dest;

    while (*dest)
      dest++;
    while (*dest++ = *src++)
      ;
    return rdest;
}

Sure, this does take another pointer's worth of space for the rdest return value, but I think that's a fine trade-off.

Also note that you can't legally define a function called strcat() in ordinary application code; that whole namespace (public functions with names starting with str) is reserved for the implementation.

unwind
  • 391,730
  • 64
  • 469
  • 606
3

dest needs to have enough memory to accommodate the concatenation in this implementation. In this implementation, it would have to be allocated by the caller. You should also be sure both dest and src are null terminated. If dest does not have enough memory, this will overwrite memory that could be used by something else.

Michael Dean
  • 1,506
  • 9
  • 11
0

Allocate enough memory for the destination string.. ie atleast (length of source string + 1).

Achuthananda MP
  • 39
  • 1
  • 2
0

Its working fine with me, I have check it.

    #include "stdio.h"


    char *strcat(char *dest, const char *src)

    {

    size_t i,j;

    for (i = 0; dest[i] != '\0'; i++)

        ;

    for (j = 0; src[j] != '\0'; j++)

        dest[i+j] = src[j];

    dest[i+j] = '\0';

    return dest;

}


void main(void)

{

    char a[10]={"abc"}, b[10]={"def"};

    strcat(a,b);

    printf("%s",a);

    getchar();

}
Siddiqui
  • 7,662
  • 17
  • 81
  • 129