1

I am trying to make my simple strcat function for learning C++.

My functions prints okay, but my main prints garbage. What am I doing wrong?

I'm still pretty new to C++, any bit of simplification and corrections are greatly appreciated. Thanks!

#include <iostream>

using namespace std;

int strlen(char* str)
{
  int len = 0;

  for (int i = 0; str[i] != '\0'; ++i) {
    ++len;
  }

  return len;
}

char* strcat(char* dest, char* src)
{
    const size_t len = strlen(dest) + strlen(src);
    char cat[len];

    for (int i = 0; dest[i] != '\0'; ++i) {
        cat[i] = dest[i];
    }

    for (int i = strlen(dest), j = 0; src[j] != '\0'; ++i, ++j) {
        cat[i] = src[j];
    }

    cat[len] = '\0';

    cout << "strCat " << cat << endl;
    return cat;
}

int main()
{
   char c1[100], c2[100];

   cout << "First string -> ";
   cin >> c1;

   cout << "Second string -> ";
   cin >> c2;

   cout << "Concatenated -> " << strcat(c1, c2) << endl;
}
Kevin
  • 327
  • 6
  • 17
  • 1
    You're returning a pointer to a local variable, `cat` is no longer valid as soon as the function returns. – user657267 Sep 29 '15 at 06:58
  • 1
    Two other problems: You forget to allocate space for the terminator, hence you have a buffer-overflow. Secondly, C++ doesn't actually have [variable-length arrays](http://en.wikipedia.org/wiki/Variable-length_array) so the declaration of `cat` is technically illegal. – Some programmer dude Sep 29 '15 at 06:59

1 Answers1

3

You are returning a pointer to a local variable cat that goes out of scope at the end of the function. This is undefined behaviour and must be avoided.

If you are trying to recreate the normal strcat function, you actually just want to put src on the end of dest and then return dest. strcat doesn't allocate any memory and assumes that dest is big enough. This mean you don't need cat at all.

Note: You can return dest as it is a pointer that is pointing to something that has a lifetime outside of the function.

The Dark
  • 8,453
  • 1
  • 16
  • 19