HiveBrain v1.2.0
Get Started
← Back to all entries
patterncModerate

strcat implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
strcatimplementationstackoverflow

Problem

I've written my own implementation of strcat in C.

char* my_strcat(char *str1, char *str2)
{
   int len_str1 = 0, len_str2 = 0, len_str3; 

   for (int i = 0; str1[i] !='\0'; i++)
   {
      len_str1++;
   }

   for (int i = 0; str2[i] != '\0'; i++)
   {
      len_str2++;
   }

   len_str3 = len_str1 + len_str2;
   char* str3 = malloc(len_str3*sizeof(char));

   for (int i = 0; i < len_str1; i++)
   {
      *(str3+i) = *(str1+i);
   }

   for (int i = 0; i < len_str2; i++)
   {
      *(str3+i+len_str1) = *(str2+i);
   }

   return str3;
}


I'm looking for anything that could be improved: readability, efficiency, code cleanup, bugs, ...

Solution

-
The most obvious is that although this matches the prototype of the standard strcat function, it works very differently. The standard strcat function does not allocate memory.

-
You do not account for the terminating NUL character when allocating memory for the new string.

-
Following from (2), you do not actually NUL terminate the new string.

-
The value of sizeof(char) is defined to be 1, so you don't really need to multiply by that value.

-
Your use of spaces is slightly inconsistent, in the first loop there is no space after != but there is in the second loop.

-
Although the use of *(str3+i) is valid and not wrong, it is more readable and common to use str3[i] instead. MISRA compliant functions have to use the latter.

-
You could move the declaration of int len_str3 down to where its value is computed, so you would have int len_str3 = len_str1 + len_str2;. That way you don't have what might appear to be an uninitialised variable at the top.

Context

StackExchange Code Review Q#149812, answer score: 15

Revisions (0)

No revisions yet.