CODEWITHSUNDEEP
cstrcat
User
User

Reputation: 99

implementing strcat without changing the inputs

I want to create an implementation of the C strcat function to concatenate 2 strings without modifying either input string. This is what I have so far

char *my_strcat(char* s1, char* s2)
{
    char* p = malloc(strlen(s1) + strlen(s2) + 1);

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

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

    *p++ = '\0';

    return p;
}

I want to populate p with all the characters in s1 and s2, but this code just returns nothing. Could use some help.

Upvotes: 0

Views: 600

Answers (4)

HostileFork says dont trust SE
HostileFork says dont trust SE

Reputation: 33627

I want to populate p with all the characters in s1 and s2, but this code just returns nothing. Could use some help.

You start with a malloc'd pointer, then are incrementing p as you go along.

At the end of the routine, what would you expect this p to point at?

Going with this approach, you'd need to remember the pointer you had malloc'd and return that.

You might find that if you give your variables more meaningful names--at least when starting out--you can reason about it better.

Also, since you're not modifying the inputs, you should mark them as const. This communicates your intention better--and gives the compile-time check for what you are actually trying to accomplish. It's especially important if you're going to be reusing a name like strcat which has existing expectations. (Reusing that name is another thing you might reconsider.)

char *my_strcat(const char* s1, const char* s2)
{
  char* result = malloc(strlen(s1) + strlen(s2) + 1);

  // To satisfy @P__J__ I will expand on this by saying that
  // Your interface should document what the behavior is when
  // malloc fails and `result` is NULL.  Depending on the
  // overall needs of your program, this might mean returning
  // NULL from my_strcat itself, terminating the program, etc.
  // Read up on memory management in other questions.

  char* dest = result;
  while (*s1 != '\0')
     *dest++ = *s1++;
  while (*s2 != '\0')
      *dest++ = *s2++;
  *dest++ = '\0';
  return result;
}

Upvotes: 2

0___________
0___________

Reputation: 68004

Another answer. This time no without explicit temporary char pointer to save the original one. And const correct types + malloc check.

Much more compiler optimizer friendly :)

#include<stdio.h>
#include<stdlib.h>
#include <string.h>


static inline char *strcpys(char *dest, const char *src)
{
    while(*dest++ = *src++);
    return dest;
}

char *mystrcat(const char *str1, const char *str2)
{
    char *result = malloc(strlen(str1) + strlen(str2) + 1);

    if(result)
    {
        strcpys(strcpys(result, str1) - 1, str2);
    }
    return result;
}

int main()
{
    char *str1 = "12345";
    char *str2 = "ABCDE";
    char *dest;

    printf("%s + %s = %s\n", str1, str2, (dest = mystrcat(str1, str2)) ? dest : "malloc error");

    free(dest);
    return 0;
}

Upvotes: -1

kiran Biradar
kiran Biradar

Reputation: 12742

Since you are incrementing the p in the concatenation process.

*p++ = *s1++;

and 

*p++ = '\0'; //don't do p++ here

p will be pointing to to beyond it's allocated memory after concatenation.

Just add one dummy pointer pointing to start of p and return it.

Please find sample code below. 

char *my_strcat(const char* s1,const char* s2)
{
  char *p = malloc(strlen(s1) + strlen(s2) + 1);

  char *start = p;
  if (p != NULL)
  {
       while (*s1 != '\0')
       *p++ = *s1++;
       while (*s2 != '\0')
       *p++ = *s2++;
      *p = '\0';
 }

  return start;
}

Upvotes: 3

Gaurav
Gaurav

Reputation: 1600

Your are moving pointer *p itself hence even the data is copied, it (pointer p) has already been placed ahead, so instead doing that make another pointer to the memory do that:

char *my_strcat(char* s1, char* s2)
{
     char* p = malloc(strlen(s1) + strlen(s2) + 1);
     char *c=p;    //RATHER THAN POINTER P, POINTER C WILL TRAVEL/MOVE
     while (*s1 != '\0')
          *(c++) = *(s1++);
     printf("%s\n\n",p);
     while (*s2 != '\0')
          *(c++) = *(s2++);
     *c = '\0';
     return p;
}

So in this case pointer p still remains at its original position, pointing to the beginning of the memory space.

Upvotes: 2

Related Questions