I`m experimenting with linked list and trying to understand pointers better. Im trying to get data from file to linked list. My file.txt looks like this:
&&&
Carl
18
male
&&&
John
22
male
&&&
Jessica
19
female
&&&
Brandon
33
male
&&&
Ema
23
female
I want it to load to linked list properly. Also number of entries showed 1 entry and also it prints only once.
My code:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <inttypes.h>
#include <math.h>
#define DATABASE_FILE "./file.txt"
#define BUFFER_SIZE 255
struct info {
char name[50];
char age[99];
char sex[99];
struct info *next;
};
typedef struct info info_t;
void load_linked_array(info_t **head) {
FILE *file;
char buffer[BUFFER_SIZE];
int counter = 0, number_entries = 0;
info_t *ptr = NULL, *current = NULL;
file = fopen(DATABASE_FILE, "r");
if (!file) { printf("Couldnt read data.!\n"); return; }
while (fgets(buffer, BUFFER_SIZE, file) != NULL) {
switch (counter ) {
case 0:
ptr = (info_t *)malloc(sizeof(info_t *));
if (!ptr) {
printf("Nepodarilo sa alokovat pamat!\n");
return;
}
break;
case 1:
strcpy(ptr->name, buffer);
ptr->name[sizeof(ptr->name)-1] = '\0';
break;
case 2:
strcpy(ptr->age, buffer);
ptr->age[sizeof(ptr->age)-1] = '\0';
break;
case 3:
strcpy(ptr->sex, buffer);
ptr->sex[sizeof(ptr->sex)-1] = '\0';
break;
case 4:
ptr->next = NULL;
if (*head == NULL) {
*head = ptr;
current = ptr;
} else {
current->next = ptr;
current = current->next;
}
number_entries ;
}
}
printf("Loaded %d entries.\n", number_entries);
fclose(file);
}
void print_linked_array(info_t **head) {
if (!head) { return; }
if (head != NULL) {
info_t *ptr;
ptr = *head;
if (ptr != NULL) {
printf("Name: %s", ptr->name);
printf("Age: %s", ptr->age);
printf("Sex: %s\n", ptr->sex);
ptr = ptr->next;
}
}
}
I tried something with counter for example like this:
case 4:
ptr->next = NULL;
if (*head == NULL) {
*head = ptr;
current = ptr;
} else {
current->next = ptr;
current = current->next;
}
number_entries ;
counter = 0;
}
}
It loaded 4 entries instead of 5 and also printed only once {here is my terminal}:
load
Loaded 4 entries.
print
Name: Carl
Age: 18
Sex: male
CodePudding user response：
First, you never reset your counter. So after reading case 4 it looks for case 5, case 6, 7, 8.
Each entry has four lines, but you have five cases. The first entry will work, it will read its four lines, but then its case 4 will read the next &&& and the next entry will be off by one.
&&& 0
name 1
age 2
sex 3
&&& 4
name 0
age 1
sex 2
&&& 3
*head is only assigned to if it is null. If it isn't it is never assigned to and will not see any of the work.
You only allocate enough space for a pointer, you need to allocate the whole struct.
print_linked_array does not loop and will only ever print the first element.
With those fixed it should work. There's a few more problems.
- You don't check there's enough space to store the individual fields. Use
strdupinstead. - The file is hard coded inside the function.
- This is not a good use of double pointers.
- Use the existing
BUFSIZ.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
// You can typedef and declare a struct in one statement.
typedef struct info {
// Store pointers rather than arbitrary size strings.
// Saves memory and allows any size fields.
char* name;
char* age;
char* sex;
struct info* next;
} info_t;
// Have a function to consistently allocate and initiate your structs.
// Avoids reading garbage or walking off the end of the list.
// (Also have one to delete it and free its fields.)
info_t *info_new() {
// Allocate the complete struct, not a pointer to the struct.
info_t *info = malloc(sizeof(info_t));
info->next = NULL;
info->name = NULL;
info->age = NULL;
info->sex = NULL;
return info;
}
// Pass in the file, do not hard code it.
// Return the new array. This is not a good use of a double pointer.
info_t *info_load(const char *file) {
int counter = 0, number_entries = 0;
FILE *fp = fopen(file, "r");
if (!fp){
// perror prints a decent error message with why it failed.
perror(file);
}
// Store the head, keep the tail to work on.
info_t *tail = NULL;
info_t *head = NULL;
// Use the existing BUFSIZ constant for your I/O buffers.
char buffer[BUFSIZ];
while (fgets(buffer, BUFSIZ, fp) != NULL){
switch(counter ){
case 0:
if( !tail ) {
// First iteration, the tail is the head.
tail = info_new();
head = tail;
}
else {
// Subsequent iterations, make a new tail and add it to the old tail.
tail->next = info_new();
tail = tail->next;
}
// We just added an entry, immediately increment the number of entries.
number_entries ;
break;
case 1:
// strdup() will allocate the correct amount of memory and copy the string.
tail->name = strdup(buffer);
break;
case 2:
tail->age = strdup(buffer);
break;
case 3:
tail->sex = strdup(buffer);
// Last field, reset the counter.
counter = 0;
break;
default:
// Put in a guard against walking off the case.
fprintf(stderr, "should not reach the default case");
exit(1);
}
}
printf("Loaded %d entries.\n", number_entries);
fclose(fp);
return head;
}
// No need for a double pointer to just read the list.
void info_print(info_t *head) {
// Loop until you hit a null.
for( info_t *info = head; info != NULL; info = info->next ) {
printf("Name: %s", info->name);
printf("Age: %s", info->age);
printf("Sex: %s\n", info->sex);
}
}
int main() {
info_t *info = info_load("file.txt");
info_print(info);
}
CodePudding user response：
There are some major problems in the code:
ptr = (info_t *)malloc(sizeof(info_t *));allocates space for a pointer, not for the structure. You should write:ptr = (info_t *)malloc(sizeof(*ptr));This mistake causes undefined behavior, which you miraculously do not observe.
ptr->name[sizeof(ptr->name)-1] = '\0';is useless if the string was shorter than the destination, and undefined behavior already happened otherwise. Use this instead to strip the newline and copy the string safely:snprintf(ptr->name, sizeof(ptr->name), "%.*s", (int)strcspn(buffer, "\n"), buffer);you should get rid of the
case 4: the new node should be linked directly after storing thesexmember andcountermust be reset to0. This causes 5 lines to be read for each element, corrupting the contents of the list.in
print_linked_arrayyou should use awhileloop instead ofif (ptr != NULL). This causes at most one element to print.print_linked_arraydoes not need to receive a double pointer, just pass theheadpointer directly.*headis uninitialized in case of error and/or empty file.the file is not closed in case of allocation error.
Here is a modified version:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <inttypes.h>
#include <math.h>
#define DATABASE_FILE "./file.txt"
#define BUFFER_SIZE 255
struct info {
char name[50];
char age[99];
char sex[99];
struct info *next;
};
typedef struct info info_t;
void load_linked_array(info_t **head) {
FILE *file;
char buffer[BUFFER_SIZE];
int counter = 0, number_entries = 0;
info_t *ptr = NULL, *current = NULL;
file = fopen(DATABASE_FILE, "r");
/* always set the head pointer for error and empty file cases */
*head = NULL;
if (!file) {
printf("Couldnt read data.!\n");
return;
}
while (fgets(buffer, BUFFER_SIZE, file) != NULL) {
size_t len = strlen(buffer);
if (len > 0 && buffer[len - 1] == '\n')
buffer[--len] = '\0';
switch (counter ) {
case 0:
ptr = (info_t *)malloc(sizeof(*ptr));
if (!ptr) {
printf("Nepodarilo sa alokovat pamat!\n");
fclose(file);
return;
}
break;
case 1:
snprintf(ptr->name, sizeof(ptr->name), "%s", buffer);
break;
case 2:
snprintf(ptr->age, sizeof(ptr->age), "%s", buffer);
break;
case 3:
snprintf(ptr->sex, sizeof(ptr->sex), "%s", buffer);
ptr->next = NULL;
if (*head == NULL) {
*head = ptr;
} else {
current = current->next = ptr;
}
current = ptr;
number_entries ;
counter = 0;
}
}
if (counter != 0) {
/* free partially filled node */
free(ptr);
}
printf("Loaded %d entries.\n", number_entries);
fclose(file);
}
void print_linked_array(const info_t *head) {
while (head != NULL) {
printf("Name: %s", head->name);
printf("Age: %s", head->age);
printf("Sex: %s\n", head->sex);
head = head->next;
}
}