Strcat is making my variable in loop change-CodePudding

I have come up with a serious problem. When I use strcat in the following C code, the value of r changes and the loop runs for an unexpected number of times. I am not able to figure out whats happening. I have posted the code with the respective outputs.

strcat used:

#include <stdio.h>
#include <string.h> 
void scan_eqn (char *eqn)
{
    char temp[15] = "";
    int i, r;
    for (i = 0, r = 0; r < 11; r  , i  )
    {
        strcat (temp, eqn   i); 
        printf("r: %d ", r)
    }
    printf("\n\n\n");
} 

int main()
{
    char eq[50];
    printf("Enter the sentence: ");
    scanf("%[^\n]s", eq);
    scan_eqn(eq);
    return 0;
}

Input:

abcdef

Output:

Enter the sentence: abcdef
r: 0 r: 1 r: 0 r: 1717920769

strcat not used:

#include <stdio.h>
#include <string.h> 
void scan_eqn (char *eqn)
{
    char temp[15] = "";
    int i, r;
    for (i = 0, r = 0; r < 11; r  , i  )
    {
        //strcat (temp, eqn   i); 
        printf("r: %d ", r)
    }
    printf("\n\n\n");
} 

int main()
{
    char eq[50];
    printf("Enter the sentence: ");
    scanf("%[^\n]s", eq);
    scan_eqn(eq);
    return 0;
}

Input:

abcdef

Output:

Enter the sentence: abcdef
r: 1 r: 2 r: 3 r: 4 r: 5 r: 6 r: 7 r: 8 r: 9 r: 10

CodePudding user response：

strcat (temp, eqn i); is senseless for several reasons:

You keep piling on strings to it, so it will first be "abcdef" then "abcdefbcdef". It will quickly overflow.
You don't check if there's anything left in eqn so after a certain number of iterations you access eqn beyond the end of the string.

I also don't understand what the magic number r < 11 is supposed to do.

CodePudding user response：

The following illustrates how the buffer progresses toward overflow...

char temp[15] = "";

defines a space in memory that can contain 14 bytes of characters, and one \0 if it is to be a string. It conceptually looks like this:

|?|?|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|?|?|
    ^ temp starts here            ^ temp ends here

On the first iteration of the following loop:

for (i = 0, r = 0; r < 11; r  , i  )
{
    strcat (temp, eqn   i); 
    printf("r: %d ", r);
}

temp is populated with the contents of eqn i -> "abcdef" ( abcdef0 ) - showing null terminator)

|?|?|a|b|c|d|e|f|0|0|0|0|0|0|0|0|0|?|?|

On the 2nd iteration, temp is appended with the contents of eqn i -> "bcdef" ( bcdef0 )

|?|?|a|b|c|d|e|f|b|c|d|e|f|0|0|0|0|?|?|

On the 3rd iteration, temp is appended with the contents of eqn i -> "cdef" ( cdef0 ) which overwrites the buffer temp

 |?|?|a|b|c|d|e|f|b|c|d|e|f|c|d|e|f|0|?|
                                   |^ buffer overflow  
                                   ^ temp ends here

This will always invoke undefined behavior.

Here are a few suggestions to eliminate the magic numbers... (See comments)

void scan_eqn (char *eqn)
{
    //char temp[100] = "";
    int i, r;
    int len = strlen(eqn);//get the length of input string
    //then size your input array accordingly using `sum of consecutive sequence` 
    size_t size = (len*(len   1)/2)   1;//adding one for final null terminator
    char temp[size]; //using VLA (Variable Length Array)
    memset(temp, 0, sizeof temp);//init all to null
    for (i = 0, r = 0; r < len; r  , i  )// replace magic number with len
    {
        strcat (temp, eqn   i); 
        printf("r: %d ", r);
    }
    printf("\n\n\n");
}

One additional suggestion. In main(void), make the following changes:

//scanf("%[^\n]s", eq);// replace this...
fgets(eq, sizeof eq, stdin);//with following two lines.   
eq[strcspn(eq, "\n")] = 0;//eliminate newline