patterncModerate
strcat implementation
Viewed 0 times
strcatimplementationstackoverflow
Problem
I've written my own implementation of strcat in C.
I'm looking for anything that could be improved: readability, efficiency, code cleanup, bugs, ...
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
-
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
-
Your use of spaces is slightly inconsistent, in the first loop there is no space after
-
Although the use of
-
You could move the declaration of
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.