C. Szurgot
C. Szurgot

Reputation: 21

When I run this code with valgrind is executes differently that when I run it without valgrind

I am attempting to create a HTML tag balancer using a stack. The program reads through a HTML file character by character and whenever it encounters a open tag, it adds it to the stack. If it encounters a close tag, it tries to remove it from the stack. The code compiles correctly however, when I execute it with valgrind, it produces a different and more correct output than when I run it without valgrind. For example:

<html>
<body>

<h1>My First Heading</h1>

<p>My first paragraph.</p>
<p>Hello

</body>
</html>

When I run the program on the previous file without valgrind it exits giving this message:

Error:
The stack is not empty at the end of the file
Stack:

However, when I run this program on that same file with valgrind it exits giving this different and correct message:

Error:
Line Number:9
The close tag, body, doesn't match the most recent open tag, p

Here is the source:

    #include<stdio.h>
    #include<stdlib.h>
    #include<stdbool.h>
    #include<assert.h>
    //#define DEBUG
    struct node
    {
            char* datum;
            struct node* next;
    };
    typedef struct node Node;
    struct stack
    {
            Node* top;
    };
    typedef struct stack Stack;
    bool isEmpty(Stack* s);
    char* pop(Stack* s);
    void push(Stack* s, char* newItem);
    char* peek(Stack* s);
    void printStack(Node* s);
    size_t size(Stack* s);
    Node* makeNode(char* x);
    void freeStack(Stack** s);

    int lineNumber = 1;

    int main(int argc, char** argv)
    {
            Stack * s = malloc(sizeof(Stack));
            s -> top = NULL;
            char* tag;
            int size;
            if(argv[1] == NULL)
            {
                printf("ERROR: Filename is NULL...Bailing");
                exit(1);
            }
            FILE* htmlFile = fopen(argv[1], "r");
            char ch;
            //int lineNumber = 1;
            while((ch = fgetc(htmlFile)) != EOF && ch != '\n')
            {

            }
            if(ch == EOF)
            {
                printf("The file id not valid");
                free(s);
                free(tag);
                fclose(htmlFile);
                exit(3);
            }

            while((ch = fgetc(htmlFile)) != EOF)
            {
                //printf("Line Number: %d\n", lineNumber);
                if(ch == '\n')
                {
                    lineNumber++;
                }
                if(ch == '<')
                {
                    tag = malloc(64*sizeof(char));
                    size = 0;
                    while((ch = fgetc(htmlFile)) != EOF && ch != ' ' &&
                        ch != '>' && ch != '/')
                    {
                        //printf("ch = %c\n", ch);
                        tag[size] = ch;
                        size++;
                        if(ch == '\n')
                        {
                            lineNumber++;
                        }
                    }
                    if(ch == ' ')
                    {
                        //Read through the rest of the tag and see if it is self-closing
                        while((ch = fgetc(htmlFile)) != EOF &&
                            ch != '>' && ch != '/')
                        {
                            //Don't care about the rest of the tag
                        }
                        if(ch == EOF)
                        {
                            //print ERROR
                            printf("Error:\nLine Number: %d\nFile ended during the middle of a tag\n", lineNumber);
                            free(s);
                            free(tag);
                            fclose(htmlFile);
                            exit(2);
                        }
                        else if(ch == '>')
                        {
                            //Add tag to stack
                            push(s, tag);
                        }
                        else if(ch == '/')
                        {
                            //Don't worry about it
                        }
                    }
                    else if(ch == '>')
                    {
                        //Add to stack
                        push(s, tag);
                    }
                    else if(ch == '/')
                    {
                        //If size == 0 read the rest of the tag and remove it
                        //Else dont worry about it
                        if(size == 0)
                        {
                            while((ch = fgetc(htmlFile)) != EOF && ch != ' ' &&
                                ch != '>')
                            {
                                //printf("ch = %c\n", ch);
                                tag[size] = ch;
                                size++;
                                if(ch == '\n')
                                {
                                    lineNumber++;
                                }
                            }
                            tag[size] = '\0';
                            //printf("Tag: %s\n", tag);
                            if(ch == EOF)
                            {
                                //Print ERROR
                                printf("Error:\nLine Number: %d\nFile ended during the middle of a tag\n", lineNumber);
                                free(s);
                                free(tag);
                                fclose(htmlFile);
                                exit(1);
                            }
                            else if(ch == ' ' || ch == '>')
                            {
                                printf("Tag: %s, Top: %s\n", tag, peek(s));

                                //Remove tag from stack or ERROR
                                if(strncmp(peek(s), tag, size+1) == 0)
                                {
                                    //Remove tag from stack
                                    pop(s);
                                }
                                else
                                {
                                    //Print ERROR
                                    printf("Error:\nLine Number:%d\nThe close tag, %s, doesn't match the most recent open tag, %s\n", lineNumber, tag, peek(s));
                                    freeStack(&s);
                                    free(tag);
                                    fclose(htmlFile);
                                    exit(3);
                                }
                            }
                        }
                    }
                    else if(ch == EOF)
                    {
                        //Print ERROR
                        printf("Error:\nLine Number: %d\nFile ended during the middle of a tag\n", lineNumber);
                        free(s);
                        free(tag);
                        fclose(htmlFile);
                        exit(4);
                    }
                    //printf("Tag: %s\n", tag);
                    //printStack(s->top);
                    //printf("\n");
                    free(tag);
                }
                printStack(s->top);
                printf("\n");
            }
            if(!isEmpty(s))
            {
                printf("Error:\nThe stack is not empty at the end of the file\nStack:");
                printStack(s->top);
                printf("\n");
                free(s);
                fclose(htmlFile);
                exit(5);
            }
            else
            {
                printf("The file is valid and all tags are closed.\n");
            }

            free(s);
            fclose(htmlFile);
            //free(htmlFile);
            //free(tag);
            exit(0);
            return 0;
    }

Upvotes: 2

Views: 647

Answers (2)

C. Szurgot
C. Szurgot

Reputation: 21

The problem came from my freeing of "tag" at the end of the while loop right before I checked if the list was empty. Since I stored the address in the stack, I was accidentally removing it from the stack when I freed the memory block.

Upvotes: 0

Schwern
Schwern

Reputation: 164629

The problem is your code has undefined behavior and memory corruption. Once that happens, all expectations of consistent behavior across various compilers and runtimes (such as valgrind) are off.

If you turn on warnings (-Wall) you'll find you have some undefined behavior.

test.c:52:18: warning: variable 'tag' is uninitialized when used here [-Wuninitialized]
            free(tag);
                 ^~~
test.c:34:18: note: initialize the variable 'tag' to silence this warning
        char* tag;
                 ^
                  = NULL

You're also failing to load string.h for strncmp.

test.c:143:32: warning: implicitly declaring library function 'strncmp' with type 'int (const char *,
      const char *, unsigned long)' [-Wimplicit-function-declaration]
                            if(strncmp(peek(s), tag, size+1) == 0)
                               ^
test.c:143:32: note: include the header <string.h> or explicitly provide a declaration for 'strncmp'

There's probably more undefined behavior in the various stack functions.

You're also freeing the stack all over the place, and in different ways, that's probably leading to freeing the same pointer multiple times. And you're often failing to free s->top and all the nodes it points and and they point to.

Once you have undefined behavior and memory corruption, all bets are off. Strange things are going to happen according to the quirks of the compiler and its memory management.

Valgrind introduces its own virtual machine and memory management to execute your code. This will interpret any undefined behavior differently.

Fix all your warnings, and fix all the memory complaints Valgrind has. Then the code should work consistently with or without Valgrind.


As a rule of thumb, every struct should come with a function to create and destroy it. This encapsulates taking care of all the various states a struct might be in, and also taking care to destroy any memory attached to it. So make Stack_new, Stack_destroy, Node_new, and Node_destroy and never malloc or free a Stack otherwise.

For example...

void Node_destroy(Node *node) {
    if( node->datum != NULL ) {
        free( node->datum );
    }
    if( node->next != NULL && node->next != node ) {
        Node_destroy(node->next);
    }
    free(node);
}

void Stack_destroy(Stack *stack) {
    if( stack->top != NULL ) {
        Node_destroy( stack->top );
    }
    free(stack);
}

Upvotes: 1

Related Questions