Reputation: 21
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
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
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