How to remove segmentation fault (core dumped) error while implementing strcat?

Question

9I am learning pointers from K&R. I was trying to implement strcat (ubuntu, gcc). The code is compiling but when I run it, i get the segmentation fault error. I searched the web for it, but all i could know is that I am "trying to access a memory location that I am not supposed to or allowed to access". But, I could not find the error in the code.

#include <stdio.h>

void xstrcat(char *s, char *t);

int main(void) {
    char *s = "hel";
    char *t = "lo.";
    xstrcat(s, t);
    printf("%s",s);
    return 0;
}

void xstrcat(char *s, char *t) {
    while(*s)
            s++;
    while(*s++ = *t++)
            ;
}

Please tell me where exactly is the error and why?

`char *s = "hel";` There is no extra space to combine the string. change to like `char s[32] = "hel";` — BLUEPIXY, Sep 19 '16 at 02:50
It looks like you need to learn the basics of how strings work before trying to implement strcat. For example you can't write to the end of s without having memory allocated there. There's a lot of other things wrong. I think if you want to learn how to program in C you need another book, not K&R. It's too hard to learn from. — sashang, Sep 19 '16 at 03:29

score 0 · Answer 1 · edited May 23 '17 at 10:27

You've actually got a couple of problems here. You're defining your strings s and t as string literals. In most implementations, string literals are read-only and cannot be modified; attempting to do so is undefined behavior. See this question. Instead, you should define your variables like this:

char s[] = "hel";
char t[] = "lo.";

However, this still won't fix your problems. s has 4 bytes allocated for it (the 3 letters and NULL terminator). Writing data beyond this length is undefined behavior. If you want to append data to s, you'll need to declare it with a size that can fit the concatenation.

char s[32] = "hel";  // now there is enough space to concatenate "lo." to the end
char t[] = "lo.";

score 0 · Answer 2 · answered Sep 19 '16 at 04:00

Understand that char s1[] = "string"; and char* s2 = "string" are not exactly the same thing, even with the close association between pointers and arrays in C. s1 is an array of characters, s2 is a pointer to a literal string that may be stored in a readonly section of the executable. Modifying s2 content will lead your program to crash.

See C FAQ 1.32 item:

http://c-faq.com/decl/strlitinit.html

score 0 · Answer 3 · answered Sep 19 '16 at 04:10

The problem isn't with xstrcat(), but with the data you're passing it. Only the first argument is at issue as the second one isn't modified and should be marked as such (const) in the function declaration. The programmer is responsible for making sure the first argument can be modified and has enough extra space in it to contain whatever is being concatenated onto it:

#include <stdio.h>

void xstrcat(char *s, const char *t);

int main(void) {
    char s[1024] = "hel";
    char *t = "lo.";
    xstrcat(s, t);
    printf("%s\n", s);
    return 0;
}

void xstrcat(char *s, const char *t) {
    while (*s != '\0') {
        s++;
    }

    while ((*s++ = *t++) != '\0') {
        /* pass */
        }
}

You would have seen this if you had read the man page for strcat() before trying to reimplement it yourself. You would also have seen something along the lines of:

The strcat() function is easily misused in a manner which enables malicious users to arbitrarily change a running program's functionality through a buffer overflow attack.

Avoid using strcat(). Instead, use strncat()

And you would have instead, as a better exercise, implemented xstrncat()

How to remove segmentation fault (core dumped) error while implementing strcat?

3 Answers3