0

This is my first post. I am trying to copy characters from one pointer to the end of another however a segmentation fault is caused. What is the appropriate way to write this?

char *my_strcat(char *dest, char *src) {
    while (*dest != '\0')
        dest++;

    while (*src != '\0'){
        *dest = *src;
        dest++;
        src++;
    }
    *dest = '\0';

return dest;
}

int main(void) {
    char *p = "Hello";
    char *q = "Carl";

    printf("%s\n", my_strcat(p, q));
}

The only error that it causes is a segmentation fault.

Thank you!

2 Answers2

1

Below are the corrections and points to remember.

  1. char *p = "Hello"; - This is never a good idea when doing string manipulations. This can have undefined behavior when modifying the values at p. Look here
  2. char *p = "Hello"; - This creates an array of size 5. If you are planning for sting concatenation, after concatenation the pointer is "supposed" to point to an array of size 5(hello)+4(carl)+1(\0)=10, which clearly is the reason for segfault. ALWAYS HAVE EXTRA SPACE IN THE ARRAY when doing string concatenation.
  3. Also, you are returning the dest pointer after incrementing it to point to the next cell after your string. You should remember the starting of the pointer, and return it.

Below is the code which can solve your problem:

#include <stdio.h>

char *my_strcat(char *dest, char *src) {

    // Save the beginning of the pointer before incrementing it.
    char *beg = dest;

    while (*dest != '\0')
        dest++;

    while (*src != '\0'){
        *dest = *src;
        dest++;
        src++;
    }
    *dest = '\0';

    // Return the beginning of the pointer
    return beg;
}

int main(void) {
    // Have a char array with more space
    char p[10] = "Hello";
    char *q = "Carl";
    printf("%s\n", my_strcat(p, q));
    return 0;
}
NVS Abhilash
  • 574
  • 7
  • 24
0

There are few problems in your code:

You are returning the address of '\0' So the output would be blank evenif you don't have a the segmentation fault.

Coming to the question,

char *p ="Hello" Defines a string literal,

Which is stored in the text segment of the program address space by the compiler.

.text segment is read only and in the code you are trying to write to something readonly.

#include<stdio.h>
void my_strcat(char *dest, char *src) {
    while (*dest != '\0')
        dest++;

    while (*src != '\0'){
        *dest = *src;
         dest++;
         src++;
    }
    *dest = '\0';
}

int main(void) {
    char p[10] = "Hello";
    char q[] = "Carl";
    my_strcat(p,q);
    printf("%s\n",p);
}

The above code fixes the issue. char p[10] ="Hello" will solve your problem

Christina Jacob
  • 665
  • 5
  • 17
  • This still won't work, as `char p[] = "Hello"` would still create an array of size 6, not 10, which is the reason of segfault. – NVS Abhilash Jul 26 '19 at 06:14
  • Updated based on the suggestion, Accepting defining bigger array is the right thing to do. But that doesn't give segmentation fault. Writing to read only string was the reason for segmentation fault. – Christina Jacob Jul 26 '19 at 09:29
  • Thanks for the replies! I didn't know a pointer acted like an array[] when declared w/a string constant, allotting a fixed size in RAM based on its length. I figured you could append to the pointer memory address by incrementing and the compiler would cope with this at run time. Perhaps another workaround would be since the length of both strings is known at compile time to use malloc on a pointer to concatenate them to via copying provided, as mentioned, the beginning address is stored to return? I will keep playing around. I feel like I have a good grasp on most C concepts except pointers... –  Jul 27 '19 at 02:17