Folks, I have been told that there is one logical problem with this code. Basically, this is an implementation of the library function strcat().

The intended beahviour of this function to concatenate ct to the end of s. Return s.

I could not find any logical problems with the code, apart from the fact that the final string should be NULL terminated, but I am not sure that can be classified as an error.

Could you help me find the error, if there is any.

char *strcat(char *s, const char *ct)
{
int i, start;
start = strlen(s);
for (i = 0; i < strlen(ct); i++)
{
s[start + i] = ct[i];
}
return s;
}

I can't see any obvious error there, except the lack of null termination, which is a big bug.

This line is very inefficient (but it will work):

for (i = 0; i < strlen(ct); i++)

A better way is do only call strlen once before the loop.

your logical error is here

for (i = 0; i < strlen(ct); i++)// this care only for the lenght of ct
{
s[start + i] = ct[i];//  start+i(s) is not greater than array size
}

I am in relearning process, sorry if i make a mistake

Not sure about this...

If you start using strlen () on a not finished string - you may get a crash.

: Not sure about this...
:
: If you start using strlen () on a not finished string - you may get
: a crash.

strlen is used on ct only, which is copied and remains untouched by the function. If it was used on the string that is built up, there would be issues.

There would perhaps also be an issue if both strings point at the same place in memory, but that is the responsibility of the caller to avoid. Of course, both strings need to be null terminated, but that is also the responsibility of the caller.

: your logical error is here
:
:

: 
: for (i = 0; i < strlen(ct); i++)// this care only for the lenght of ct
: {
: s[start + i] = ct[i];//  start+i(s) is not greater than array size
: }
: 

:


No that looks fine, start is used as an offset to determine where the "s" string ends.

s[5]= "cat"
a[5] = "and"


call this function with s a

strcat(s , a)

this function will do

s = "catand";
wouldn't that be greater than the 5 and will be writting in other memory allocation. That is my point of view. strncat is a better choose to never write more than you have to.

*** correction
s[start + i] = ct[i];// start+i is not greater than array size of s
****************
I am in relearning process, sorry if i make a mistake

Yes that is a problem, but it isn't strcat()'s problem, it is something the caller must solve.

Hi alls,

Support that there is no problem with the size of s, we should add: s[start + i] = '\0'; after the for loop as follow:

char *strcat(char *s, const char *ct)
{
   int i, start;
   start = strlen(s);
   for (i = 0; i < strlen(ct); i++)
      s[start + i] = ct[i];
   
   s[start + i] = '\0';
   
   return s;
}

Regards.

How about this?

<I'm still learning>

char *strcat(char *s, char *ct)

it is alright but it is not good as the above.

why? the second parameter should not be changed inside the function. so, we should declare it as a const parameter.