Mizushima Hideyoshi
Mizushima Hideyoshi

Reputation: 49

undefined behavior when strcat

in = "(A + B) * C - D * F + C";
#define MAXL 256

I'm having a problem with my code at case ')'.

My code is unfinished since it misses a few lines of code somewhere to add all the final char = operators inside stack to tempExp which I'll probably figure out soon™. What I need at the moment is your input with why this line while(c[0] != '(') strcat(tempExp, c); would result in undefined behavior.

Thank you very much!

Note: the reason for this messy code c[0] = *((char*)pop(s)) is that pop returns a void* which I cannot change for the purpose of this exercise.

void convertIntoPost(char * in, char ** out)
{
    int i;
    Stack * s = createStack();
    char tempExp[MAXL] = "temp: ", c[2];
    c[1] = '\0';
    printf("\n%s\n", tempExp);
    for(i = 0; i <= strlen(in); ++i)
    {
        printf("i: %d", i);
        c[0] = in[i];
        if(isalpha(c[0]) || isalnum(c[0]))
        {
            c[0] = in[i];
            printf("\nc passed isalpha OR isalnum: %s\n", c);
            strcat(tempExp, c);
        }
        else
        {
            switch(in[i])
            {
                case ' ' : break;
                case '(' :
                    push(s, &in[i]);
                    break;
                case ')' :
                    c[0] = *((char*)pop(s));
                    printf("c in case ')': %s", c); /* Show expected result */
                    printf("\n%s", tempExp); /* Just checking tempExp and see no problem */
                    while(c[0] != '(')
                        strcat(tempExp, c);
                    printf("\n%s", tempExp); /* Program stopped before it gets here */
                    break;
                default :
                    while(!isEmpty(s) && (priority(in[i]) <= priority(c[0] = *((char*)pop(s)))))
                        strcat(tempExp, c);
                    push(s, &in[i]);
            }               
        }            
    }
    printf("\nThis is in: %s", in);
    printf("\n%s", tempExp);
    *out = (char*)malloc(strlen(tempExp) + 1);
    *out = strdup(tempExp);
    makeEmpty(s);
}

int priority(char c)
{
    if(c == '(')
        return(0);
    else if(c == '+' || c == '-')
        return(1);
    else if(c == '*' || c == '/')
        return(2);
}

Upvotes: 0

Views: 449

Answers (2)

Some programmer dude
Some programmer dude

Reputation: 409196

Besides the infinite loop pointed out by Grantly, you also have several other problems, mostly because of two things:

  1. Your loop includes the string terminator of in, so you will end up in the default case in the switch statement where you will call, in effect, priority('\0') which leads to your second problem. You will also call push with a pointer to the string terminator, and if push assumes that the pointer is not to the end of the string you might have problems there too.

  2. The second problem is with the priority function, because it does not return a value for all branches leading to undefined behavior if you pass a character not expected in your conditions (like for example the string terminator). You need to add an explicit else clause.

Upvotes: 4

Grantly
Grantly

Reputation: 2556

The loop

while(c[0] != '(')
   strcat(tempExp, c);

will run indefinitely (assuming c[0] is not '('), overrunning the size of your string (256 chars), as it will keep adding the same string (c) onto the tempExp. This will cause a myriad of errors, but usually a stack overflow or buffer overflow ... Undefined behaviour only when the end of the string (256) is reached, from then on it will crash unelegantly

Upvotes: 1

Related Questions