CODEWITHSUNDEEP
clinked-list
Obamaself
Obamaself

Reputation: 29

C LinkedList storing the last value only

I am having problem storing all the values into the Generic LinkedList, my linkedlist works totally works on a normal user Keyboard input but when I try to store values(strings) from a file, there is something weird happening, it only store the last value of the file.

I have checked my addToList() function but theres nothing wrong with it.

P.s But I am feeling its either I am printing wrong or my reading from the file into the linkedlist is wrong.

Thank you. 

 #include<stdio.h> 
#include <stdlib.h>
#include<string.h>
#include "LinkedListItems.h"
#define MAX 10000

int main()
{
    printf("Testing MissileFIle.txt");

    void* secondStr;
    //Had to malloc the thing 
    secondStr = (void*)malloc(1*sizeof(char));
    FILE* missileFile;
    missileFile = fopen("missiles.txt", "r");

    if(missileFile == NULL)
    {
        printf("The file is empty");
    }    

    number_list_t* missileList = calloc(1, sizeof(number_list_t));

    void* input;

    //Have to allocate the input 
    input = malloc(1*sizeof(void*));

    //this is to read the data into the second Str
    while(fgets(secondStr,MAX,missileFile) != NULL)
    {
        //Let just print out first just to test my memory
        printf("%s\n",secondStr);
        //Right now its only reading one string so far which is really weird AFFFFF
        addTolist(missileList,secondStr);

    }

    //Gotta declare another list just to print out the list 
    number_node_t* current = missileList->head; 


    while(current != NULL)
    {
        //There is something wrong with this line
        printf("%s\n",current-> number);
        current = current-> next; 
    }
    fclose(missileFile);



}

OUTPUT: Testing MissileFile.txt

splash

single

V-line

h-line

Single Single Single Single Single Single

typedef struct NumberNode 
{
    //It can store any data type 
    void* number;
    struct NumberNode* next; 
}number_node_t;


//List of Nodes 
typedef struct NumberList 
{
    number_node_t* head; 
    int count; //This is not nesssary but it can be useful for counting how  many variables 

}number_list_t;




       void addTolist(number_list_t* list, void* newNumber)
    {
        //tem[ = newNode]
        number_node_t* newNode = calloc(1,sizeof(number_node_t));
        newNode->number = newNumber; 
        newNode->next = list->head;
        list->head = newNode;
    }

INPUT DATA: single splash single V-Line h-line Single

Upvotes: 0

Views: 235

Answers (3)

user3629249
user3629249

Reputation: 16540

regarding:

while(fgets(secondStr,MAX,missileFile) != NULL)

MAX is defined as 10000 but secondStr is defined as pointer to one byte. so when this is executed, a buffer overflow occurs.

This is undefined behavior and probably the root of the problem with reading from a file

Upvotes: 0

chux
chux

Reputation: 153348

At least this issue:

Copy the string

OP's goal includes the need to copy the string from the read buffer to the list, not just copy the buffer pointer.

// void addTolist(number_list_t* list, void* newNumber) {
void addStringTolist(number_list_t* list, const char *s) {
    // number_node_t* newNode = calloc(1,sizeof(number_node_t));
    number_node_t* newNode = calloc(1, sizeof *newNode); // todo: add error check
    size_t sz = strlen(s) + 1;
    newNode->number = malloc(sz);  // todo: add error check
    strpy(newNode->number, s);   
    newNode->next = list->head;
    list->head = newNode;
}

Note: When freeing the list, newNode->number also needs to be free'd.

Upvotes: 0

Desperados
Desperados

Reputation: 434

The way you have implemented this, it cannot work.

The main problem, among many, is related to the void* pointers which cannot be dereferenced. The size of elements should be given, either on creating the list in which case all elements are of the same type, or separately for each individual element. You can check out this question for an example of something that could work.

As far as the buffer thing is concerned, addToList should allocate new memory for each newNumber. What you are currently doing results in all data of the list pointing to a specific space in memory (the one allocated to secondStr). Each time you change the content of that memory space, all elements in the list are affected. This is why you print the same value for all elements and more specifically the last value in your file.

The way you allocate memory is also not really ok, same goes for the way you open your file, there is memory leaking etc. I am not going into details.

Upvotes: 1

Related Questions