0

I need to create a function to concatenate 2 strings, in my case they are already given. I will need to concatenate the strings 'hello' and 'world!' to make it into 'helloworld!'. However, I can't use library functions besides strlen(). I also need to use malloc. I understand malloc would create n amounts of bytes for memory, however, how would I make it so that it can return a string array if thats possible.

Here is what I have so far,

#include <stdio.h>
#include <string.h>
int *my_strcat(const char* const str1, const char *const str2)
{   
    int s1, s2, s3, i = 0;
    char *a;

    s1 = strlen(str1);
    s2 = strlen(str2);
    s3 = s1 + s2 + 1;

    a = char *malloc(size_t s3);

    for(i = 0; i < s1; i++)
        a[i] = str1[i];

    for(i = 0; i < s2; i++)
        a[i+s1] = str2[i];

    a[i]='\0';

    return a;
}

int main(void)
{
    printf("%s\n",my_strcat("Hello","world!"));
    return 0;
}

Thanks to anyone who can help me out.

2
  • 2
    Returning int * is wrong Commented Oct 22, 2016 at 3:20
  • strlen() returns size_t not int. Commented Oct 22, 2016 at 8:06

3 Answers 3

3

This problem is imo a bit simpler with pointers:

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

char *mystrcat(char *a, char *b) {
  char *p, *q, *rtn;
  rtn = q = malloc(strlen(a) + strlen(b) + 1);
  for (p = a; (*q = *p) != '\0'; ++p, ++q) {}
  for (p = b; (*q = *p) != '\0'; ++p, ++q) {}
  return rtn;
}

int main(void) {
  char *rtn = mystrcat("Hello ", "world!");
  printf("Returned: %s\n", rtn);
  free(rtn);
  return 0;
}

But you can do the same thing with indices:

char *mystrcat(char *a, char *b) {
  char *rtn = malloc(strlen(a) + strlen(b) + 1);
  int p, q = 0;
  for (p = 0; (rtn[q] = a[p]) != '\0'; ++p, ++q) {}
  for (p = 0; (rtn[q] = b[p]) != '\0'; ++p, ++q) {}
  return rtn;
}
Sign up to request clarification or add additional context in comments.

3 Comments

Nice. Very concise.
@adabsurdum, Gene Firstly thanks for a clean & efficient snippet. Quick question. Can I call this snippet in this way? char *rtn = mystrcat("Hello ", "world!"); rtn = mystrcat(rtn, " Good Morning"); // expecting result to be like "Hello world! Good Morning"; will that have any over memory consumption/leakage ?
Yes if that's all you do. Every call allocates a new buffer for the result. Two calls require to free()s. nb: This style where a function mallocs space for its return value should be avoided if possible. It's a bad pattern exactly because it's so easy to cause memory leaks.
3

Here is an alternate fix. First, you forgot #include <stdlib.h> for malloc(). You return a pointer to char from the function my_strcat(), so you need to change the function prototype to reflect this. I also changed the const declarations so that the pointers are not const, only the values that they point to:

char * my_strcat(const char *str1, const char *str2);

Your call to malloc() is incorrectly cast, and there is no reason to do so anyway in C. It also looks like you were trying to cast the argument in malloc() to size_t. You can do so, but you have to surround the type identifier with parentheses:

a = malloc((size_t) s3);

Instead, I have changed the type declaration for s1, s2, s3, i to size_t since all of these variables are used in the context of string lengths and array indices.

The loops were the most significant change, and the reason that I changed the consts in the function prototype. Your loops looked fine, but you can also use pointers for this. You step through the strings by incrementing a pointer, incrementing a counter i, and store the value stored there in the ith location of a. At the end, the index i has been incremented to indicate the location one past the last character, and you store a '\0' there. Note that in your original code, the counter i was not incremented to indicate the location of the null terminator of the concatenated string, because you reset it when you looped through str2. @jpw shows one way of solving this problem.

I changed main() just a little. I declared a pointer to char to receive the return value from the function call. That way you can free() the allocated memory when you are through with it.

Here is the modified code:

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

char * my_strcat(const char *str1, const char *str2)
{   
    size_t s1, s2, s3, i = 0;
    char *a;

    s1 = strlen(str1);
    s2 = strlen(str2);
    s3 = s1+s2+1;
    a = malloc(s3);

    while(*str1 != '\0') {
        a[i] = *str1;
        str1++;
        i++;
    }
    while(*str2 != '\0') {
        a[i] = *str2;
        str2++;
        i++;
    }

    a[i] = '\0';                    // Here i = s1 + s2

    return a;
}


int main(void)
{
    char *str = my_strcat("Hello", "world!");
    printf("%s\n", str);

    /* Always free allocated memory! */
    free(str);

    return 0;
}

4 Comments

sizeof(char) is always unnecessary and a bad idea.
@ChrisDodd-- you're right. I was trying to be clear and pedagogical with the OP, who seemed to have some troubles with malloc(), but it really wasn't helpful. I have fixed this in my answer.
Make all those ses a size_t and you could drop the ugly cast to s3 when passing it to malloc().
@alk-- That is a great suggestion. Done. I should have thought of that myself.
0

There are a few issues:

In the return from malloc you don't need to do any cast (you had the syntax for the cast wrong anyway) (see this for more information).

You need to include the header stdlib.h for the malloc function.

And most importantly, a[i]='\0'; in this i is not what you need it to be; you want to add the null char at the end which should be a[s3]='\0'; (the length of s1+s2).

This version should be correct (unless I missed something):

#include <stdio.h>
#include <stdlib.h> //for malloc
#include <string.h>

char *my_strcat(const char* const str1, const char *const str2)
{
    int s1,s2,s3,i=0;
    char *a;
    s1 = strlen(str1);
    s2 = strlen(str2);
    s3 = s1+s2+1;
    a = malloc(s3);
    for(i = 0; i < s1; i++) {
        a[i] = str1[i];
    }
    for(i = 0; i < s2; i++) {
        a[i+s1] = str2[i];
    }
    a[s3-1] = '\0'; // you need the size of s1 + s2 + 1 here, but - 1 as it is 0-indexed

    return a;
}


int main(void)
{
    printf("%s\n",my_strcat("Hello","world!"));
    return 0;    
}

Testing with Ideone renders this output: Helloworld!

15 Comments

Shouldn't the return type be char * instead of int * ?
@SandeepTuniki Oh right, missed that. I think you are correct (although it might not matter in this case given that char might be an unsigned int, although I think it is implementation defined)
Oh, I see. I actually had a bit of trouble understanding how to use malloc tbh. Thanks so much!
WRONG! This is an off by one error. You should allocate s3 + 1 because the write of buffer[s3] is incorrect.
@self Is it? Please tell me where? I'm guessing that you mean a[s3]='\0'; but it already has +1 for the null char. Look at the assignment to s3: s3 = s1+s2+1;
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.