1

I'm basically coding my very own string functions in C.

I've been trying to do the strcat function using pointers and cannot seem to understand whether I should be allocating memory using malloc or leaving it up to the heap.

char *my_strcat(const char *s1, const char *s2)
{


    const unsigned char *p1 = (const unsigned char *)s1;
    const unsigned char *p2 = (const unsigned char *)s2;

    unsigned char *string;
        //string = malloc(strlen(s1) + strlen(s2) + 1);
    while (*p1 != '\0')
{
        *string = *p1;
        string++;
        p1++;

        if(*p1 == '\0')
        {
            while(*p2 != '\0')
            {
                *string = *p2;
                string++;
                p2++;
            }
        }
    }
    return (char *)string;  
}

Any tips on more efficiently performing this task or things I'm doing wrong would be great!

Cheers

EDIT

OK so I got a working solution but just wondering after I use malloc where should I free() it? 

char *my_strcat(const char *s1, const char *s2)
{


    const unsigned char *p1 = (const unsigned char *)s1;
    const unsigned char *p2 = (const unsigned char *)s2;

    char *string = malloc(sizeof(char *));
    char *res = string;

    while (*p1 != '\0')
{
        *string = *p1;
        string++;
        p1++;
    }
    while (*p2 != '\0')
    {
        *string = *p2;
        string++;
        p2++;
}
    *string = '\0'; 

    return (char *)res; 
}
eddie
  • 1,252
  • 3
  • 15
  • 20
Alistair Gillespie
  • 540
  • 1
  • 5
  • 22
  • 1
    you'd be better off implementing strncat... strcat() is a major major source of stack overflow holes in apps. – Marc B Aug 29 '12 at 03:49
  • 2
    How do you get a Stack Overflow from strcat()? Buffer overruns I understand, but... – mimicocotopus Aug 29 '12 at 03:53
  • `string` needs to point to the end of `p2`. PS not initializing `string` does not "leave it to the heap". – obataku Aug 29 '12 at 03:57
  • 1
    @MarcB: the standard `strncat()` function has an interface designed to trigger errors; it is appalling. – Jonathan Leffler Aug 29 '12 at 04:07
  • @JonathanLeffler I am appending a string to the end of a destination string. I think if you look closer I am roughly doing the right as this works with arrays... I asked for tips... – Alistair Gillespie Aug 29 '12 at 05:37
  • I've withdrawn my comment. You've got an unusual loop structure; it's not the way it is normally written. You need to think rather carefully about what happens after you've appended the second string to the first. Where does `p1` point? (Hint: it isn't at the `'\0'` that would terminate the loop.) – Jonathan Leffler Aug 29 '12 at 05:42

3 Answers3

3

First, I assume that the allocation is commented out by mistake.

  • You need to save the pointer that you allocate, and return it. Otherwise, you're returning a pointer string, which points at the end of the concatenation result
  • You are not terminating the resultant string; you need to add *string = '\0'
  • You should move the second loop to the outside of the first loop, and drop the if condition around it: if the first loop has terminated, you know that *p1 points to \0
char *string = malloc(strlen(s1) + strlen(s2) + 1);
char *res = string;
for (; *p1 ; *string++ = *p1++);
for (; *p2 ; *string++ = *p2++);
*string = '\0';
return res;
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • 5
    The standard `strcat()` function does no memory allocation. – Jonathan Leffler Aug 29 '12 at 04:17
  • Also: malloc can return NULL, so it might be a good idea to check for that. If you get a warning for the `char *string = malloc...` line, an explicit cast to `(char*)` will make it go away. – Nathan Andrew Mullenax Aug 29 '12 at 05:02
  • @Nathan http://stackoverflow.com/questions/953112/should-i-explicitly-cast-mallocs-return-value http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc – Alok Singhal Aug 29 '12 at 05:05
  • @Alok. Solid. Nice catch. To summarize the links: the warning is from not including stdlib.h. Void pointers are automatically promoted to pointer to whatever. – Nathan Andrew Mullenax Aug 29 '12 at 05:16
  • @JonathanLeffler I understand that the standard `strcat` does not allocate memory, but it looks like the "OP's very own" `my_strcat` was going to, so I went along with the OP on that. – Sergey Kalinichenko Aug 29 '12 at 10:59
  • @NathanAndrewMullenax That is true, `malloc` returns must always be checked for `NULL`. The code in the answer is not a production-level code, though - it is only a skeletal implementation intended to illustrate the main idea. – Sergey Kalinichenko Aug 29 '12 at 11:02
2

strcat doesn't allocate any memory so if you're trying to accomplish the same thing then you don't need to use malloc.

char* strcat(char* destination,char* source) {
 int c = 0;
 int sc;

 while(destination[c] != 0) {  c++; }

 for(sc = 0;sc < strlen(source);sc++) {
  destination[sc+c] = source[sc];
 }

 destination[sc+c] = 0;

 return destination;

}
mimicocotopus
  • 5,280
  • 4
  • 22
  • 24
0

This worked for me.

char* my_strcat(char* a,char* b)
{
        int i,j;

        for(i=0;a[i];i++);
        for(j=0;b[j];j++,i++)
         a[i]=b[j];
        a[i]='\0';

}
Ankuraho
  • 1