I am very much a beginner in C so I know my code may look like crap. All I wanted to do was make a linked list with a struct that holds an array of chars and int for frequency. It reads lines from a test file and simply prints out the file. It reads the file correctly, iterates over the linked list the first time and prints it out correctly. However, on the second iteration of the linked list, it prints out the correct number of lines and the correct integer, but the words are replaced by symbols. I have looked everywhere and I just need some help. Here is my code.
#include <stdio.h>
#include <stdlib.h>
#define Word_MAX_LENGTH 255
struct WordFreq
{
char word[Word_MAX_LENGTH];
int frequency;
struct WordFreq *next;
} WordFreq;
struct WordFreq *head = NULL;
void insert(struct WordFreq *newNode)
{
if (head == NULL)
{
head = newNode;
newNode->next = NULL;
return;
}
struct WordFreq *current = head;
while (current->next != NULL)
{
current = current->next;
}
current->next = newNode;
newNode->next = NULL;
}
void main(int argc, char *argv[])
{
if (argc != 2)
{
printf("Please run as %s [filename]\n", argv[0]);
return;
}
FILE *f;
f = fopen(argv[1], "r");
if (f == NULL)
{
printf("File (%s) does not exist\n", argv[1]);
return;
}
struct WordFreq *line = malloc(sizeof(struct WordFreq));
if (line == NULL)
{
printf("Cannot do dynamic memory managment\n");
return;
}
printf("File content in %s:\n", argv[1]);
while (fscanf(f, "%s %d", line->word, &(line->frequency)) != EOF)
{
printf("%s %d\n", line->word, line->frequency);
insert(line);
line = malloc(sizeof(struct WordFreq));
if (line == NULL)
{
printf("Cannot do dynamic memory management");
return;
}
}
fclose(f);
// To keep head intact for sorting portion
struct WordFreq *current = head;
printf("\nContent of linked list:\n");
while (current != NULL)
{
printf("%s %d\n", current->word, current->frequency);
struct WordFreq *temp = current;
current = current->next;
free(temp);
}
printf("\n\n\n");
current = head;
while (current != NULL)
{
printf("%s %d\n", current->word, current->frequency);
current = current->next;
}
free(current);
free(line);
}
Very appreciative of any help. Quite sad that I have been programming a while, but I want to learn C because I want to learn the ins and outs of why programs do what they do to help for future endeavors.
You free the linked list during the first iteration, so the second iteration has undefined behavior because head has become an invalid pointer and the contents of the memory of the list nodes have changed. Reading from them has undefined behavior, anything can happen including the program stopping with a invalid memory access or random output as you observe.
You should write functions: one to print the list contents and a separate one to free the list. Combining these operations creates confusion and leads to logical errors such as this one.
Also note these remarks:
the return type of main is int. Modify the return statements to return 1 on error and 0 on success, ie: normal operation.
printf("File (%s) does not exist\n", argv[1]) might not be the correct diagnostic: the file might exist but fopen will fail if the process does not have the proper access priviledges. You could use errno and output the appropriate error message:
#include <errno.h>
#include <string.h>
[...]
if (f == NULL) {
fprintf(stderr, "cannot open %s: %s\n", argv[1], strerror(errno));
return 1;
}
instead of fscanf(f, "%s %d", line->word, &(line->frequency)) != EOF you should test for fscanf() success at performing both conversion: it should return 2. Better even: read the input one line at a time with fgets and use sscanf() to convert the line contents and report conversion errors with an explicit message that includes the offending input line.
head is freed twice: free(current) at the end must be removed.
Here is a modified version:
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define WORD_MAX_LENGTH 255
struct WordFreq {
char word[WORD_MAX_LENGTH];
int frequency;
struct WordFreq *next;
} WordFreq;
struct WordFreq *head = NULL;
void insert(struct WordFreq *newNode) {
if (head == NULL) {
head = newNode;
} else {
struct WordFreq *current = head;
while (current->next != NULL) {
current = current->next;
}
current->next = newNode;
}
newNode->next = NULL;
}
void print_list(const struct WordFreq *node) {
while (node != NULL) {
printf("%s %d\n", node->word, node->frequency);
node = node->next;
}
}
void free_list(struct WordFreq *node) {
while (node != NULL) {
struct WordFreq *temp = node;
node = node->next;
free(temp);
}
}
int main(int argc, char *argv[]) {
char line[WORD_MAX_LENGTH + 20];
if (argc != 2) {
fprintf(stderr, "Please run as %s [filename]\n", argv[0]);
return 1;
}
FILE *f = fopen(argv[1], "r");
if (f == NULL) {
fprintf(stderr, "Cannot open %s: %s\n", argv[1], strerror(errno));
return 1;
}
printf("File content in %s:\n", argv[1]);
while (fgets(line, sizeof line, f)) {
struct WordFreq *node = malloc(sizeof(*node));
if (node == NULL) {
fprintf(stderr, "Cannot allocate node for %s\n", line);
fclose(f);
return 1;
}
if (sscanf(line, "%254s %d", node->word, &node->frequency) == 2) {
printf("%s %d\n", node->word, node->frequency);
insert(node);
} else {
fprintf(stderr, "invalid line: %s\n", line);
free(node);
}
}
fclose(f);
printf("\nContent of linked list:\n");
print_list(head);
// ...
free_list(head);
return 0;
}