strcat(); printing extra characters

Question

I have made strcat() function myself but after adding the string it is printing an extra ascii symbol. Please tell why?

  #include<stdio.h>
  #include<conio.h>
  #include<string.h>
  void xstrcat(char string1[],char string2[]);
  void main(void)
{ char x[100];
  char string1[40],string2[40];
  printf("Enter a string:");
  gets(string1);
  puts("Enter another string:");
  gets(string2);
  xstrcat(string1,string2);
  printf("%s",string1);
  getch();
}
  void xstrcat(char string1[],char string2[])
{
  int i,x,y;
  x=strlen(string1);
  y=strlen(string2);
  for(i=0;i<y;i++)
  { string1[i+x]=string2[i];
  }//for ends
}

@fahad: there's a little button at the top of the box where you write your question that looks like `101` \n `010`. Please use it on your code in the future. It seems like every question you ask has to be reformatted by someone else. — Cogwheel, Jul 02 '10 at 18:55
Tangential to your question, but don't use `gets`: http://stackoverflow.com/questions/2843073/warninggets-function-is-dangerous . Even its own documentation says not to use it. — Tyler McHenry, Jul 02 '10 at 18:56
Could be the `void main`, it has been known to provoke *undefined behavior*. Better switch to the correct `int main` soon. — Thomas Matthews, Jul 02 '10 at 20:02
@fahad, using `fgets` is equally easy and does not have fatal bugs with no workarounds. — R.. GitHub STOP HELPING ICE, Jul 03 '10 at 10:34
You don't use the variable 'x'; you don't check that `gets()` returns successfully; (as already noted, you should not use `gets()` - use `fgets()` instead, but watch for the newline); you do not check that the data will fit in the space allocated; (as already noted, you should use the standard form of `main()` which returns an `int`). You don't use enough compiler warnings - because some of these would be reported to you if you did. And when debugging code, it is often a good idea to print out (echo) the values read - so print `string1` and `string2` before calling `xstrcat()`. — Jonathan Leffler, Jul 03 '10 at 18:13

score 4 · Accepted Answer · answered Jul 02 '10 at 18:55

4

Your xstrcat() function isn't placing a null terminator character at the end of the resulting string.

One possible fix might be to put the following just before the xstrcat() function returns:

string1[x+y] = '\0';

answered Jul 02 '10 at 18:55

Michael Burr

333,147
50
533
760

I dint add \0 myself because my teacher told me that it is added itslef you dont need to add it.Any suggestion for that?i did as you said any now the program runs fine :) – Jul 03 '10 at 10:07
Added itself when doing what? A string literal in C automatically includes 0 termination, and library functions like strcpy, strcat, etc. will write the 0 byte. But here your're manipulating the memory byte-for-byte yourself. How could the language possibly change the following byte to 0 without being horribly wrong in the general case? – R.. GitHub STOP HELPING ICE Jul 03 '10 at 10:36

score 2 · Answer 2 · answered Jul 02 '10 at 18:59

In C, strings are terminated with a NUL byte (character with value 0). strlen will tell you how many characters there are from the beginning of the string until the NUL byte, not counting the NUL byte itself.

So when you execute this loop:

for(i=0;i<y;i++)
  { string1[i+x]=string2[i];
  }

You never copy the terminating NUL byte from string2 into string1, and so string1 no longer has a terminating NUL (you overwrote its NUL earlier in the loop with the first character of string2). When a string is missing its terminating NUL, functions that read it (e.g. printf) will continue reading past the string's intended endpoint until they do eventually find a NUL byte somewhere further along in memory. This can lead to printing extra characters and/or a crash.

Either change y to y+1 or explicitly insert a '\0' at position x+y in string1.

score 1 · Answer 3 · answered Jul 02 '10 at 19:40

1

void xstrcat(char string1[],char string2[])
{
  //int i,x,y;
  size_t i, x, y;
  // They could also be unsigned, but size_t is an unsigned big enough to hold the
  // biggest in memory index possible

  x=strlen(string1);
  y=strlen(string2);

  //for(i=0;i<y;i++)
  for (i=0; i<=y; i++) // This picks up the null at the end
  { string1[i+x]=string2[i];
  }//for ends
}

Alternately you could do this as:

void xstrcat(char * string1, const char * string2)
{
   while(*string1) {
       string1++;
   }
   strcpy(string1, string2);
}

This should be a little faster because it doesn't have to traverse either string but once. It also doesn't require as many extra variables.

answered Jul 02 '10 at 19:40

nategoose

12,054
27
42

Your alternate version is no better; neither version traverses either string more than once. Another alternate version is simply `strcpy(string1+=strlen(string1),string2);` but I think the point of the question was how to implement strcat from the ground up rather than based on other library functions. – R.. GitHub STOP HELPING ICE Jul 03 '10 at 10:38
@R: strlen traverses the string to count the characters, and copying a string requires a traversal. The first version traversed string2 once for length and once for copying and string1 only once for lenght, while the second only traversed string1 once for length (via finding its end) and string2 was traversed only once where both length and copying were done (as long as strcpy was implemented as copy until null). – nategoose Sep 10 '10 at 21:40

score 0 · Answer 4 · answered Jul 03 '10 at 07:13

your loop

for (i = 0; i < y; i++) {
    string1[i+x]=string2[i];
}

you are reading string 2 up to its length-1 which will read only upto the last character before NULL, if you dont know in c or c++ strings are terminated with a NULL ( 0 )character.

So your string1 will not have a NULL character in the end.

So now when you try to print your string1 it will definitely print some characters in the end as your string was of size 40, these characters are garbage.

Mostly you will get a 40 character string printed except only when on of these garbage values is 0.

So you should read the second string upto 0, so make your loop

for (i = 0; i <= y; i++)

or you can simply add a NULL character in the end of string 1 after adding string2 into it.

string1[x+y] = 0;

strcat(); printing extra characters

4 Answers4