Array concatenation function is returning gibberish.

Question

This involves two functions, both of which are listed below:

char *catw(char *s1, char sep, char *s2) 
{
  char s[strlen(s1) + strlen(s2) + 1];
  for(int i = 0; i < strlen(s1); i++) {
    s[i] = s1[i]; 
  }
  s[strlen(s1)] = sep;
  for(int j = 1; j <= strlen(s2); j++) {
    s[j + strlen(s1)] = s2[j]; 
  }
  char *rs = s;
  return rs;
}

The above function works fine, it takes two strings and concatenates them together using the character in between them.

char *catw_arr(char *ss[], char sep)
{
  char *ar = ss[0];
  for(int i = 1; i < strlen(ss); i++)
  {
    ar = catw(ar, sep, ss[i]);
    printf("%s\n", ar);

  }
  return ar;
}

This function, however, is the problem. It is supposed to take an array of strings, with an int that is the length of the array, and a separator, and concat all the strings together. The printf is in there because I wanted to test what was going on.

here's the main function I'm using:

int main()
{
  char *abc[3] = {"a", "b", "c"};
  printf("%s\n", catw_arr(abc, 3, '/'));
  return 0;
}

This is what it churns out:

a/
?ĶS?/
?ĶS?/

I honestly have no idea what the problem is here. I'm assuming it's concatenating the first string with the separator, but then it's running into some crap that it can't process and starts outputting gibberish.

Your `catw` returns a pointer to a local variable. There's no telling what will happen to that space after control passes out of the function. — Beta, Jan 29 '17 at 23:25
How many times do you wanna call `strlen` on the same string? Geez. — DeiDei, Jan 29 '17 at 23:26
Also `s` length is off by 1. You need to reserve space for both the separator and the null terminator in addition to both strings. — Phil1970, Jan 29 '17 at 23:29

score 2 · Answer 1 · answered Jan 29 '17 at 23:25

2

The function catw is actually your problem there. You are defining a s variable as an array. This variable, and the storage associated to it, are only valid for as long as the function runs.

When you copy s to rs, you simply make a copy of the adress, not of the associated storage. Once your function ends, both s and rs point to freed memory, and any access to them will be undefined behaviour. Because they still point to stack memory, you don't get a segmentation fault, but you are reading data that has been since replaced by something else.

answered Jan 29 '17 at 23:25

Rémi Bonnet

793
5
16

Okay, thanks! Is there any way I can change catw to fix this, or should I just replicate the code in that function within catw_arr? – jazaniac Jan 29 '17 at 23:36
See the linked question in the duplicate header. – Rémi Bonnet Jan 29 '17 at 23:38
You cannot returns a variable on the stack... and thus major changes would be required to the code. And by the way, as someone point out, your code is very inefficient as you call `strlen` in a loop which is a total waste of CPU cycles. Thus given the allocation and inefficiency problem it does not make much sense to reuse the code as most code would be different. Any competent programmer would computed the total length only once for the function that receive an array. Obviously some code sharing is possible but not using your function. And the code that can be reuse exist in C librairie. – Phil1970 Jan 29 '17 at 23:50

Array concatenation function is returning gibberish.

1 Answers1