I am trying to initialize a linked list from a .txt file using strtok()
.
But when I want to initialize the name (the first element of my structure) strtok returns a "(null)".
However when I printf()
my strElement
I get the expected name.
heals.c
#include "heals.h"
ListeHeals* initHeals()
{
ListeHeals* ListeHeals = malloc(sizeof(*ListeHeals));
char const* const fileName = "items/heals.txt";
FILE* file = fopen(fileName, "r");
if (file == NULL)
{
printf("Fichier non ouvert");
}
char line[256];
const char * separator = "|";
int count = 0;
while (fgets(line, sizeof(line), file)) {
char* strElement = strtok (line, separator);
while (strElement != NULL) {
Heals* heal = malloc(sizeof(*heal));
if(count == 0)
{
printf("%s\n", strElement);
heal->name = strElement;
}
else if(count == 1)
{
heal->heal = atoi(strElement);
ListeHeals->first = heal;
}
strElement = strtok (NULL, separator);
count = 1;
}
count = 0;
}
fclose(file);
return ListeHeals;
}
void printListeHeals(ListeHeals* ListeHeals)
{
if (ListeHeals == NULL)
{
exit(EXIT_FAILURE);
}
Heals* actual = ListeHeals->first;
while (actual != NULL)
{
printf("Nom : %s\n", actual->name);
printf("heal : %d\n\n", actual->heal);
actual = actual->next;
}
printf("NULL\n");
}
The output The first line is my printf.
Here is the file heals.txt :
Potion de vie I|30
Potion de vie II|80
Potion de vie III|200
The heal structure (heal.h):
#ifndef heals_h
#define heals_h
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
struct Heals
{
char* name;
int heal;
struct Heals* next;
};
typedef struct Heals Heals;
struct ListeHeals
{
struct Heals* first;
};
typedef struct ListeHeals ListeHeals;
ListeHeals* initHeals();
void printListeHeals(ListeHeals* ListeHeals);
#endif
The output I expect:
nom : Potion de vie I // I have (null)
heal : 30 // I already have it
Any help is welcome thank you!
CodePudding user response:
There are multiple problems and you are doing things more complex than needed.
Most importantly:
- You do not enqueue your nodes into your list.
- You do not reset
count
for each new line. This means you will not handle the name for any other line. - You do not allocate memory for your strings but only assign pointers into your
line
buffer. That will go out of scope when the function is done. - As you allocate new nodes for each token you will end up with one node holding the name and one node holding the other value.
Update: Why did this cause the result you got? You allocate one node for the name but do not enqueue it in your list. Then you allocate another node. This node does not hold the name but only the second value. This node goes into the list. When you print the content of your list you will only find the second node.
Also you overwrite the head of your list with any new node without linking them together.
Therefore you will only get 1 node in your list, no matter how many lines you read.
And this node will only hold the heal
value.
A fixed version could look like this (untested):
ListeHeals* initHeals()
{
ListeHeals* ListeHeals = malloc(sizeof(*ListeHeals));
// TODO: Check for NULL
char const* const fileName = "items/heals.txt";
FILE* file = fopen(fileName, "r");
if (file == NULL)
{
printf("Fichier non ouvert");
// TODO: return with some error indication. You mustn't continue the function.
}
char line[256];
const char * separator = "|";
while (fgets(line, sizeof(line), file)) {
char* strElement = strtok (line, separator);
if (strElement != NULL) {
Heals* heal = malloc(sizeof(*heal));
// TODO: Check for NULL
// Handle the name
printf("%s\n", strElement);
heal->name = malloc(strlen(strElement 1);
strcpy(heal->name, strElement);
// Handle the value
strElement = strtok (NULL, separator);
// TODO: Check for NULL
heal->heal = atoi(strElement);
// enqueue node into front position
heal->next = ListeHeals->first;
ListeHeals->first = heal;
}
else
printf("invalid file content: %s\n", line);
}
fclose(file);
return ListeHeals;
}
CodePudding user response:
There are several problems here, most of which @Gerhardh has already detailed in their answer. But the one that actually causes the problem you asked about is
- You create a new
Heals
on each iteration of your inner loop, set a value for only one member of each one, and assign each as the head of your list. Thus, when you get around to printing, theHeals
at the list head (that being the only one then in the list) has had only itsheal
member set, not itsname
element. A differentHeals
gets the corresponding name set, but that one is then leaked.
It looks like Gerhard's revised code fixes that, too. Even though they didn't enumerate this particular issue, their more natural implementation approach isn't prone to making such mistakes.
CodePudding user response:
I tried and that output this :
Potion de vie I
Potion de vie II
Potion de vie III
Nom : Potion de vie III
heal : 0
Nom : Potion de vie II
heal : 0
Nom : Potion de vie I
heal : 0