Home > Net >  C linked list problem, segfault when printing the list to a file
C linked list problem, segfault when printing the list to a file

Time:08-11

I'm trying to write and use linked list. When trying to print the list to the file the first string gets chained to the last node and it causes a segfault.

I've tried debugging and it led me nowhere.

It only happens using printListToFile(...) and readstl(...).

The code

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "lists_c.h"

#define test "ijm digidimdimadam jjsklva /s4t\t \nmjam \nla kookaracha la kookaracha\n"
/*a method that creates a "blank" node, declares a the next node and points it to NULL, the char array is already initialized.*/
struct Node *makeNode();
struct Node *makeFullNode(struct Node *Next, char *Ptr);
struct Node *readFile(char *path);
struct Node *readstl(char*);
void printList(struct Node *head);
void insertToList(struct Node *node, char *str);
void printListToFile(char *path, struct Node *head);

int main(int argc, char *argv[])
{
    struct Node *head;
    head = (struct Node *)malloc(sizeof(struct Node));
    printf("this is mmn 23 Q3, a func that stores a file in a linked list and then prints it.\n");/*
    printf("%s\n",argv[1]);
    if (argc == 2) {    */
        head = readFile("tester1.txt");
        printList(head);
        printListToFile("test.tmp", head);
        insertToList(head->next, test);
        printf("head");
        free(head);
    /*}
    else if (argc > 2) {
        printf("Too many arguments supplied.\n");
    } else {
        printf("One argument expected.\n");
    }*/
    return 0;
}

struct Node *makeNode()
{
    struct Node *node;
    node = (struct Node *)malloc(sizeof(struct Node));
    if (node == NULL) {
        printf("memory allocation failed\n");
        return NULL;
    }
    node->next = NULL;
    return node;
}

/*a method that creates a node with all values, declares a the next node and points it to the next node recieved
and changes the char array to the one recieved.*/
struct Node *makeFullNode(struct Node *Next, char Ptr[])
{
    struct Node *node;
    node = (struct Node *)malloc(sizeof(struct Node));
    node->next = (struct Node *)malloc(sizeof(struct Node));
    node->ptr[NICE] = Ptr[NICE];
    node->next = Next;
    return node;
}

/*the method that reads the file into the list into the char array of each node and returns the head of the list */
struct Node *readFile(char *path)
{
    FILE *fptr;
    struct Node *curr, *head;
    curr = (struct Node *)malloc(sizeof(struct Node));
    head = (struct Node *)malloc(sizeof(struct Node));
    if (curr == NULL || head == NULL) {
        printf("memory allocation failed\n");
        return NULL;
    }
    curr = head;
    fptr = fopen(path,"r");
    if (fptr == NULL) {
        printf("error - failed to open file\n");
        exit(0);
        return NULL;
    }
    while (fgets(curr->ptr, NICE, fptr))/*if fgets returns NULL it means that we either reached EOF or error, both a reason to end the loop*/
    {
        curr->next = makeNode();
        curr = curr->next;  
    }
    if(ferror(fptr))/*checking if the loop ended for the right reason*/
    {
        printf("error - failed to read file\nthis is what we got so far\n");
    }
    printf("file succesfully read\n");
    fclose(fptr);
    free(curr);
    return head;
}

/*the method that reads the file into the list into the char array of each node and returns the head of the list */
struct Node *readstl(char *path)/*!!!problem!!!*/
{
    FILE *fptr;
    int i, len;
    struct Node *head;
    head = (struct Node *)malloc(sizeof(struct Node));
    fptr = fopen("readstrtofile.tmp", "w");
    if (fptr == NULL) {
        printf("error - failed to open file\n");
        exit(0);
        return NULL;
    }   
    len = strlen(path);
    for (i = 0; i < len && fputc(*(path   i), fptr) != EOF; i  );
    if (ferror(fptr))/*checking if the loop ended for the right reason*/
    {   
        printf("error - failed to read into file\nthis is what we got so far\n");
    }
    fclose(fptr);
    head = readFile("readstrtofile.tmp");
    remove("readstrtofile.tmp");
    return head;
}

/*a method that prints the recieved list depending on the list to have the equal length rows.
as specified in mmn 12 tab creating uneven rows is acceptable therefor when ever there is tab in the file
the rows will appear uneven as to tab takes a place of one char but prints to 8  blank spaces in ubuntu   */
void printList(struct Node *head)
{
    struct Node *curr;
    curr = (struct Node *)malloc(sizeof(struct Node));
    if (head == NULL) {
        printf("empty list\n");
        return;
    }
    if (curr==NULL) {
        printf("memory allocation failed\n");
        return;
    }
    printf("this is the list printed nicely\n");
    curr = head;
    printf("%s\n", curr->ptr);
    while (curr->next != NULL) {
        curr = curr->next;
        printf("%s\n", curr->ptr);
    }
    printf("\n/********************/\n");
}

/* a method that creates a new file named path and prints a list to it */
void printListToFile(char *path,struct Node *head)/*!!!problem!!!*/
{
    struct Node *curr;
    char c;
    int i;
    FILE *fptr;
    curr = (struct Node *)malloc(sizeof(struct Node));
    if (curr == NULL) {
        printf("memory allocation failed\n");
        exit(0);
    }
    curr = head;
    fptr = fopen(path, "w ");
    if (fptr == NULL) {
        printf("error - failed to open file\n");
        exit(0);
    }
    if (head == NULL) {
        printf("empty list\n");
        exit(0);
    }
    printf("this is the file %s printed nicely\n",path);
    curr = head;
    while (curr->next != NULL) {
        printf("new node -> ptr -> %s\n",curr->ptr);
        /*if(sizeof(curr->ptr)/sizeof(char)<=NICE)
        {*/
            for(i=0;i<NICE && (c = fputc(curr->ptr[i],fptr)) != EOF;i  );   
            printf("puting %s to file %s\n",curr->ptr,path);
            curr = curr->next;
            printf("bolili\n");
        /*}
        else
        {
            printf("lilibo\n");
            break;
        }*/
    }
    printf("\n/********************/\n");
}

/* a method that recievs a string turns it into a list and inserts it into another list */
void insertToList(struct Node *node, char *str)
{
    struct Node *tail, *head;
    tail = (struct Node *)malloc(sizeof(struct Node));
    head = (struct Node *)malloc(sizeof(struct Node));
    if (tail == NULL || head == NULL) {
        printf("memory allocation failed\n");
        return;
    }
    head = readstl(str);/*reading the string to a node*/
    printList(head);
    printf("\n/***********in insert*********/\n");
    tail = head;
    while (tail->next) {
        tail = tail->next;
    }/*getting tto the last node to connect it*/
    strcpy(tail->ptr, node->next->ptr);/*connecting the lists*/
    tail->next = node->next->next;
    strcpy(node->ptr, head->ptr);
    node->next = head->next;
    printf("inserted string successfully\n");
}

/* a method that returns the size of the list*/
unsigned int sizeOfList(struct Node *head)
{
    struct Node *tmp;
    int size;
    if (!(head))
        return 0;
    size = 1;
    tmp = (struct Node *)malloc(sizeof(struct Node));
    tmp = head;
    while ((tmp = tmp->next))
        size  ;
    return size;
}

header file

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define NICE 80
#define debug "\n/****************/\n"

/*the node structure of a the list which contains a string the size we choose is nice 
to print by and a pointer to the next node in the list. */
struct Node {
    char ptr[NICE];
    struct Node *next;
};

/*makeNode - a method that returns a newely created node and sets all values to NULL values.*/
struct Node *makeNode();

/*makeFullNode - a method that return a newely created node with values set to params as listed below.
@param Next - tho ptr to the next node desired.
@param Ptr - the string that will go into the node ptr.*/
struct Node *makeFullNode(struct Node *Next, char *Ptr);

/*readFile - a method that reads a file into a linked list returning the head to that list.
it reads the file using fgets and a const to decide what size the node strings should be.
@param path - the name of the file to open.
@return the head of the list.*/
struct Node *readFile(char *path);

/*readstl - a method that reads a string into a linked list returning the head to that list.
it prints the list to a tmp file then reads the file using readFile.
@param path - the string to read.
@return the head of the list.*/
struct Node *readstl(char*);

/*printList - a method that prints the list to stdout.
it goes in loop on the nodes each time printing the node->ptr.
@param head - the head of the linked list.*/
void printList(struct Node *head);

/*insertToList - a method that inserts a string into list.
it creates a new list using readstl the connects the nodes using the basic method.
@param node - the node to override.
@param str - the string to insert.*/
void insertToList(struct Node *node, char *str);

/*a method that prints the list to stdout.
it goes in loop on the nodes each time printing the node->ptr.
@param head - the head of the linked list.*/
void printListToFile(char *path,struct Node *head);

/*sizeOfList - a method that returns how many node are in the list.
it goes in a while loop incresing counter by 1 each iteration.
@param head - the head of the list to measure.
@return the size of the list.*/
unsigned int sizeOfList(struct Node *head);

CodePudding user response:

I think it is this line:

for(i=0;i<NICE && (c = fputc(curr->ptr[i],fptr)) != EOF;i  );

Why would fputc ever return EOF if everything goes right? It will try to write characters behind the ptr array. From the manpage of fputc:

fputc(), putc() and putchar() return the character written as an unsigned char cast to an int or EOF on error.

This means, your loop writes as long as fputc doesn't return an error code. But what you probably want, is writing either 80 (NICE is not a nice constant name) or strlen(curr->ptr) characters. Assuming based on your readFile function, you want both things as a limit. I would recommend rewriting this line as:

int len = strnlen(curr->ptr, 80);
if (fwrite(curr->ptr, 1, len, fptr) < 0) {
    printf("error writing file");
}

or if the string is always null terminated:

if (fputs(curr->ptr, fptr) < 0) {
    printf("error writing file");
}

Also ptr is not a good name for an array. It might confuse people or even you after some time, trying to understand the code. A better name would be value or text (even arr would be better, because it is not a pointer).

CodePudding user response:

In readFile you have a loop in which you read a line and put it in curr->ptr and then create a new curr and link it to the previous curr.

This creates an extra empty curr at the end of the list which you don't need and so you free it later. Unfortunately, the next pointer of the previous node is still pointing at the empty node you just freed.

The easiest way to fix this without restructuring the loop a bit is to keep a record of the previous node, something like this:

struct Node* prev = NULL;
while (fgets(curr->ptr, NICE, fptr))/*if fgets returns NULL it means that we either reached EOF or error, both a reason to end the loop*/
{
    curr->next = makeNode();
    prev = curr;
    curr = curr->next;  
}

free(curr);
if (prev != NULL) 
{
    prev->next = NULL;
}
else
{
    head = NULL; // For the empty file case. head == curr in this case
}
// The other clean up stuff

  • Related