Concatenate two strings

Question

I am learning C and wrote a function to concatenate two strings in C. Please let help me improve it.

#include <stdio.h>
#include <string.h>
void concat (char result[], const char str1[], int n1, const char str2[], int n2)
{
 int i, j;
 // copy str1 to result
 for ( i = 0; i < n1; ++i )
 result[i] = str1[i];
 // copy str2 to result
 for ( j = 0; j < n2; ++j )
 result[n1 + j] = str2[j];
}
int main (void)
{
 const char s1[5] = { 'T', 'e', 's', 't', ' '};
 const char s2[6] = { 'w', 'o', 'r', 'k', 's', '.' };
 char s3[11];
 int i;
 concat (s3, s1, 5, s2, 6);
 for ( i = 0; i < 11; ++i )
 printf ("%c", s3[i]);
 printf ("\n");
 return 0;
}

• syb0rg: I'm assuming you know about the strcat() function in the standard library, so I'm tagging this as such. Please remove that if this assumption is false.
• Gardener: @syb0rg no, I didn't. But now I do?
• syb0rg: You learn something new every day ;)
• David Conrad: If you know strcat(), you should also know strncat().
• mdfst13: reinventing-the-wheel is for when someone wants to indicate that they don't want to be told about the existing library function to perform the task. It should only be added by the original poster. It allows people to deliberately implement something unnecessary. It should not be used by someone else if the poster does not know about the library function. It's inappropriate here, as most of the answers are of the form, "You could have just used strncat." Such answers would be invalid on a reinventing-the-wheel question.

Answer 1

This looks pretty good! A few notes (some stuff you may still have to learn about since you are a beginner, such as pointers)

• Your method isn't what I would consider "normal". To see what I mean, take a look at the function prototype of the standard library function strcat():

  char * strcat ( char * destination, const char * source );
  

  As you can see, the standard library only takes in two parameters, not 5. This makes sense too: as the caller I want to throw in two strings and only get one returned back to me. For a string concatenation function, I shouldn't have to manually enter in the length of the strings I want to concatenate, the function should be able to handle that.

• So how do we find the length of a string exactly? Well, we can iterate through a string until we find the 0円 character (termination character).

  while(*dest != '0円') /* find the end of the string */
  {
   dest++;
  }
  

  All this loop does it start at the beginning of the dest string and find the end of it, very simple. The next loop is a bit more complicated:

  while((*dest++ = *source++) != '0円'); /* copy source */
  

  This code iterates through the source string, and while it iterates through it, it appends the contents to dest (which we are doing by iterating dest at the same time as source). We do this until we reach the 0円 character.

• We should be returning a value from the function. This is more for a convenience to the caller. This allows me to do stuff such as

  puts(concat(strdup("test"), "this out"));
  

  Rather than this longer piece:

  char* str = strdup("test");
  concat(str, "test");
  puts(str);
  

• strcat can be dangerous because if the string to be appended is too long to fit in the destination buffer, it will overwrite adjacent memory, invoking undefined behavior. Usually the program will simply cause a segmentation fault when this occurs, but a skilled attacker can use such a buffer overflow to break into a system.

  To prevent buffer overflows, several alternatives for strcat() have been used. All of them take an extra argument which encodes the length of the destination buffer and will not write past that buffer end. All of them can still result in buffer overflows if an incorrect length is provided.

  char* strncat(char *dst, const char *src, size_t n);
  

  This will require a bit of modification to the copying for checking n, but not much.

---

Final Implementation (assuming no standard library functions can be used):

char* concat_s(char *dest, const char *src, size_t n)
{
 if (n != 0) // if n == 0 then we aren't concatenating anything... 
 {
 while (*dest != 0)
 {
 dest++;
 }
 do 
 {
 if (0 == (*dest = *src++)) break;
 dest++;
 } while (--n != 0);
 *dest = 0;
 }
 return dest;
}

Final Implementation (assuming all functions can be used except strncat()):

char* concat_s(char *dest, const char *src, size_t n)
{
 return memcpy(dest + strlen(dest), src, n);
}

• glampert: Wouldn't be a bad idea also presenting a "safer" version that takes a max size for dest. (strncat)
• syb0rg: @glampert I was just working on making an edit for that. Please hold :P
• Gardener: Thanks, your answer was very thorough. Other answers were helpful too, much appreciated.

Answer 2

You do not need to declare #include <string.h> in your program as it is.

You can improve your concat() function by changing the for loops so they can work on variable-length character strings. ie strings with different lengths, not just 5 and 6. Note you are testing for the null-terminator which is 0円

for ( i = 0; i < str1[i] != '0円'; ++i )
 result[i] = str1[i];

Generally, you will probably just need to call strcpy() and strcat(). In this case, you will need #include <string.h>

strcpy(s3,s1);
strcat(s3,s2);

Instead of using a for loop to output s3 , do:

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

Your whole program will be much smaller:

#include <stdio.h>
#include <string.h>
int main (void)
{
 const char s1[] = { "Test " };
 const char s2[] = { "works." };
 char s3[11];
 strcpy(s3,s1);
 strcat(s3,s2);
 printf("%s\n",s3);
 return 0;
}

I'm not terribly familiar with C, so perhaps other reviews will offer more useful info. However, using library functions is preferred.

• Wrox: Is this a bad review (the downvote)? I will gladly improve my answer on feedback.
• ben rudgers: I didn't down vote you. In part because I am no C expert. But it appears that the strings are not null terminated and I believe this makes strcopy and strcat poor choices. My perhaps mistaken understanding [based on Plauger's The Standard C Library] is that memcopy into a new array of type char, is safer. Of course, I could be wrong.
• Wrox: @benrudgers Thanks! I improved my answer. My bad. I forgot to change how the character strings were initialized. I thought strncpy, the "safer" version of strcpy had issues of it's own.
• ben rudgers: Again no expert but I think strncopy will struggle if the string contains non-terminating null characters. So it's really a question of what "safe" means. To me, once you've got strings without null terminators, other assumptions about null become more doubtful.

Answer 3

Interface

You use const for str1 and str2, which is good.

For n1 and n2, it is customary to use size_t as the type, as a hint to other programmers that those are array sizes. Using int isn't wrong, though.

I suggest returning result for the caller's convenience.

The purpose of the function is either confused or falsely advertised. C strings are customarily terminated with a null character ('0円'). If you name your parameter str..., then I would expect that it relies on such null terminators. On the other hand, you also require the lengths n1 and n2 to be explicitly provided. That looks like more of a memory-copying operation than a string-concatenating operation.

Side note: When some parts of a system work with null-terminated strings, while other parts work with strings of explicit lengths (and which may contain null characters as valid data), the mismatch tends to cause a whole class of bugs, some of which have security implications.

main()

Printing a character at a time is unusual and tedious. A call to printf("%.11s\n") would be much more convenient than looping.

If you wanted to print a character at a time anyway, then putchar() would be a simpler function to call.

Implementation of concat()

Declare variables in the tightest scope possible, as late as possible. To another programmer encountering your code, the int i, j declaration at the top is meaningless. On the other hand,

for (int i = 0; i < n1; ++i)

makes it clear that i is a counter that is only used in that loop.

Always write your blocks and loops with the optional braces. By omitting the braces, you are contributing to a future coding accident.

• David Conrad: Opie may or may not be using a C99+ compiler.
• 200_success: @DavidConrad The code has C99-style comments. Might as well go all the way. Besides, it's 2014 — that's 15 years since C99!