cstringstrcat

Strcat is making my variable in loop change


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

Solution

  • 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