Pointer version of strcat

Question

#include <stdio.h>

void strpcat(char *s, char *t) {
    int i = 0;
    while (*s++ != '\0')
        i++;
    while ((*(s+i) = *t++) != '\0')
        i++;
}

int main(void) {
    char a[20] = "Hello";
    char b[] = "Bye";
    strpcat(a, b);
    printf("%s\n", a);

    return 0;
}

I wanted to write rewrite the strcat() function with pointers, but when I run the program nothing changes.

Nothing affects to a array, what did I do wrong?

Answer 1

The one problem is this while loop

while (*s++ != '\0')
    i++;

If *str is equal to '\0' nevertheless the pointer s is incremented. So the original string will not be changed. At least the second string will be appended to the array after the source string.

Another problem is using the variable i in the second while loop

while ((*(s+i) = *t++) != '\0')
     i++;

because the pointer s was already incremented in the previous while loop. Using the incremented variable i and the incremented pointer s the pointer expression s + i will have an incorrect address.

The variable i is redundant.

The function can be declared and defined the following way

char * strpcat( char *s, const char *t ) 
{
    char *p = s;

    while ( *p ) ++p; 

    while ( ( *p++ = *t++ ) != '\0' );

    return s;
}

and called like

puts( strpcat( a, b ) );

Pay attention to that the second parameter should be declared with the qualifier const because the second passed string is not changed within the function. Also you could add the qualifier restrict as in the standard function strcat

char * strpcat( char * restrict s, const char * restrict t );

Answer 2

You're overcomplicating it:

void strpcat(char* s, char* t) {
  while (*s++ != '\0')
  { // do nothing
  }

  s--;   // need to go back one char so s points to the `\0`.

  while ((*s++ = *t++) != '\0')  // remove the fancy and wrong stuff
  {
     // do nothing
  }
}

Answer 3

2 steps approach:

• Go to the NULL of first string
• Continue to copy second string

void strpcat(char* s, char* t) {
  while (*s != '\0') { ++s; } // Go to the end
  while ((*s++ = *t++) != '\0') {} // Copy source to end of destination
}

Answer 4

If applying the KISS_principle:

#include <string.h>

char* kisscat (char* restrict dst, const char* restrict src)
{
  char* end = dst + strlen(dst);
  strcpy(end, src);
  return dst;
}

Now as it happens this is not only more readable, but potentially also faster than rolling any of this out manually. Since these library calls might get inlined into some very efficient code. Unlike a completely hand-made function.

Answer 5

Your implementation increments s past the null pointer, then copies the string pointed to by t well past this position to s[i] and so on. The original null terminator is not overwritten, so printf("%s\n", a) still outputs Hello and the string in copied too far into the destination array, potentially beyond the end, causing undefined behavior.

If you want to use pointers, do not also use an index and increment only if not pointing to the null pointer already.

Here is a modified version:

#include <stdio.h>

char *strpcat(char *s, const char *t) {
    while (*s != '\0')
        s++;
    while ((*s = *t++) != '\0')
        s++;
    return s;   // return a pointer to the null terminator.
}

int main(void) {
    char a[20] = "Hello";

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

    char *p = strpcat(a, " world!");

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

    strcpy(p, " Bye.");

    printf("%s\n", a);
    return 0;
}

Output:

Hello
Hello world!
Hello world! Bye.