Writing strcat (string concatenate) in C

Question

char *cat_string(char *to_cat, char *destination) {
 char *copy = destination;
 while (*copy != '0円') {
 *copy++;
 }
 while (*to_cat != '0円') {
 *copy++ = *to_cat++;
 }
 *copy = '0円';
 return destination;
}

I would like to know if this is an efficient and effective way of writing this function. It seems silly to have to iterate through the whole array just to find the 0円 character.

Also, should I be returning the pointer to the destination? What's the best practice there?

Answer 1

You renamed strcat() to cat_string(), so I suppose it's fitting that you reversed the traditional order of the parameters as well. ☺

I would make three changes:

• Changing the first argument to const helps prevent users notice if they call the function with the parameters swapped (but doesn't prevent it in all cases).
• You have a stray pointer dereference. It was misleading, but luckily harmless. (In the first while loop, you just want to advance the copy pointer. You don't care about what value it points to as you advance; you only care when you check for the NUL terminator.)
• (削除) You can remove a statement just by changing the loop structure. (削除ここまで)
  Edit (Rev 3): Previous recommendation was buggy, and has been retracted. Credit to @MarcvanLeeuwen.

To answer your questions...

• The only way to find out where to start appending is to walk the entire destination string to locate where it ends. That's just how C-style strings work.
• Yes, returning destination seems appropriate. It's analogous to what strcat() does.

One final note: two spaces for indentation is too stingy, in my opinion. It's insufficient for readability, and it also encourages inappropriately deep nesting (which would be a symptom of poorly organized code).

char *cat_string(const char *to_cat, char *destination) {
 char *copy = destination;
 while (*copy != '0円') {
 copy++; /* Removed superfluous dereference. */
 }
 ...
}

• Marc van Leeuwen: This "improvement" breaks the correct original. Changing while to do-while only changes the start of the loop, not the end: with post-increment in the assignment, the (second while) condition still tests a character yet to be copied, and most of the time the terminating null character of to_cat is not copied to destination, which then remains unterminated. A null character only gets copied in case to_cat originally pointed to a null character (zero length concatenation), but then the loop continues until (if lucky) finding a second null character.
• ratchet freak: the real strcpy is while(*copy++=*to_cat++); which will end after the final 0円
• 200_success: @MarcvanLeeuwen Are you sure?
• Marc van Leeuwen: @200_success Of course I am sure. In your final do-while loop, you copy a character from *do_cat++, then because of the post-increment the *to_cat tested in the "while" condition is the next character, not yet copied; if it is found to be null it will not have been copied. Compare this to the solution cited by rachet freak, which copies a character and then tests the same character for being null. It could somewhat more explicitly be written while((*copy++=*to_cat**)!='0円') {}
• 200_success: @MarcvanLeeuwen I stand corrected! Thank you for pointing out the error.

Answer 2

1 major issue: undefined behavior when the target doesn't have enough space to hold it all and no way to prevent it with the parameters you ask for

char *cat_string(char *to_cat, char *destination, size_t dest_size) {
 char *copy = destination;
 copy[dest_size-1]=0;//ensure 0 termination
 while (*copy != '0円') {
 copy++;
 }
 char* end_buffer = destination+dest_size-1;
 while (*to_cat != '0円' && to_cat!=end_buffer) {
 *copy++ = *to_cat++;
 }
 *copy = '0円';
 return destination;
}

this changes the semantics a bit but ensures that the buffer doesn't overflow

• 200_success: In other words, you recommend implementing strncat() instead of strcat().
• ChrisW: @200_success strncat isn't a safe strcat: strncat appends the first n bytes of source, but does nothing to avoid overflowing destination. You may be thinking of strcat_s or similar.
• 200_success: @ChrisW You're right! I hadn't even heard of strcat_s() — it seems to be a Windows-only thing.
• ChrisW: @200_success The equivalent on Unix seems to be strlcat. And maybe there's a version of strcat_s defined in C11.