Thread: I want to concatenate two strings by using pointers but get a segfault

Hello,

I ultimately want to concatenate two strings using pointers. For some reason I get a segmentation fault as I try to "debug". Can someone tell my why I get this segfault? Thanks!!

Code:

#include <stdio.h>


char* strcopy(char* str_1, char* str_2){
  int idx_2 = 0;
  int idx_1 = 0;
  char* both;
  for (; *(str_1 + idx_1) != '\0'; idx_1++){
    *(both + idx_1) = *(str_1 + idx_1);
  }
  for (; *(str_2 + idx_2) != '\0'; idx_2++){
    *(both + idx_1 + idx_2) = *(str_1 + idx_1 + idx_2);
  }
  *(both + idx_1 + idx_2 + 1) = '\0';


  return both;
}


int main(){


  char *a = "hello";
  char *b = "greetings";


  printf("%s\n", a);


  for (int i = 0; *(a + i) != '\0'; i++){
    *(a + i) = 'b';
  }


  printf("%s\n", a);


  //printf("%s\n", strcopy(a, b);


  return 0;
}

You never initialize your both variable. It is an automatic variable, so it has a basically random value. You start writing to it, without ever pointing it at something.

It's like visiting a friends house to sleep over, then in the middle of the night walking down a pitch-black hallway to the pitch-black bathroom without turning on any lights, then unzipping and starting to pee in a random direction. You have no idea where it's going, but I can guarantee that you're making a mess.

You probably should compute the length of the two strings, add one, and allocate some storage using malloc or some passed-in allocator interface. Then use that as the destination to copy into.

Also, I notice that you don't use the parameter pointers for anything. Yet you keep doing (str_1 + idx_1) as a pointer expression. Why not just advance str_1 locally: *both++ = *str_1++;

Thanks I also found out that I initialized immutable strings so overwriting would always cause a segmentation fault. I did not know about the need to always allocate memory for a variable if it should be changed.

I have another question - how can you reply in a thread without quoting? I did not find any button for that.

Originally Posted by aghast

You never initialize your both variable. It is an automatic variable, so it has a basically random value. You start writing to it, without ever pointing it at something.

It's like visiting a friends house to sleep over, then in the middle of the night walking down a pitch-black hallway to the pitch-black bathroom without turning on any lights, then unzipping and starting to pee in a random direction. You have no idea where it's going, but I can guarantee that you're making a mess.

You probably should compute the length of the two strings, add one, and allocate some storage using malloc or some passed-in allocator interface. Then use that as the destination to copy into.

Also, I notice that you don't use the parameter pointers for anything. Yet you keep doing (str_1 + idx_1) as a pointer expression. Why not just advance str_1 locally: *both++ = *str_1++;

first off - like I asked in my previous post - is there a way to reply without quoting?

I have rewritten the problem so that I concatenate two arbitrary strings that may be immutable so I think it cannot get shorter than what I have done right?

I used one printf() in strcat() for debugging to see how result looks like (it should read "hello man") but I get an empty line as output and dont understand why.

Code:

#include <stdio.h>
#include <stdlib.h>


char* strcat(char* str_1, char* str_2){
  int len_str1 = 0;
  int len_str2 = 0;
  while(*str_1++ != '\0')
    len_str1++;
  while(*str_2++ != '\0')
    len_str2++;


  char* result = (char*) malloc(sizeof(char) * (len_str1 + len_str2));


  str_1 -= len_str1 + 1;
  str_2 -= len_str2 + 1;
  while(*str_1 != '\0'){
    *result++ = *str_1++;
  }
  
  printf("%s\n", result);
  while ((*result++ = *str_2++) != '\0'){
  }
  return result;
}




int main(){
  char a[] = "hello";
  char b[] = " man";


  printf("%s\n", strcat(a, b));


  return 0;
}

Originally Posted by rstanley

For starters, strcopy() should called strconcat() as you are attempting to concatinate two strings in the function.

You have declared "both" as an uninitialized pointer, and have not allocated any data space.

"both" would need to be allocated by malloc() or calloc() to a size that is at least strlen(str_1) + strlen(str_2) + at least 1 for the Nul byte, and I would use a value more than just 1. Then it would be safe to return "both". Then free() it in main() before exiting the program.

Before altering "a" in the for loop:

Code:

char *a = "hello";

Creates a pointer to a constant string, and cannot be altered! Should be:

Code:

char a[] = "hello";

This would create a mutable array which is initialized with a constant string. Same for "b".

Code:

printf("%s\n", strcopy(a, b);

You are missing a closing ')'.

Now go back and run your strcopy() function with a debugger to see where your other mistakes are in strcopy().

Originally Posted by john_12

I have another question - how can you reply in a thread without quoting? I did not find any button for that.

At the bottom of each posting, there are two buttons, "Reply" and "Reply With Quote". Just use "Reply".

Plus there is an open box at the bottom, "Quick Reply" that is always present.