Why is my "print" function only read the last element and garbage then stop responding?

1
#include <stdlib.h>
#define NULL 0

struct student
{
    int id;
    char name[20];
    float marks;
    struct student *next
};
typedef struct student node;
void main()
{
    node *head;
    void read(node *p);
    void create(node *p);
    int count(node *p);
    void print(node *p);
    head=(node *)malloc(sizeof(node));
    read(head);
    printf("\n");
    print(head);
}
void read(node *list)
{
    FILE *fp;
    char filename[30];
    printf("input file name: ");
    scanf("%s",filename);
    fp=fopen(filename,"r");

    if(fp == NULL)
    {
        return; // file doesn't exist
    }
    while(fscanf(fp, "%s %d %f", list->name, &list->id, &list->marks) == 3)
    {
        printf("%s \t%d \t%f\n", list->name,list->id,list->marks);//check
        list->next=(node*)malloc(sizeof(node));//create next node
    }
    fclose(fp);
    return;
}
void print(node*list)
{
    if(list->next!=NULL)
       {
           printf("ID=%d\n",list->id);
           printf("Name=%s\n",list->name);
           printf("Marks=%f\n",list->marks);
           printf("\n");
           if(list->next->next==NULL)
            {
                printf("%d",list->next->id);
            }
           print(list->next);
       }
       return;
}

It looks like it printed the last line and next line(which is garbage).I think the problem is in the read function ? Since I dont feel the way I check it is accurate. Anyway i have not been able to fix this, so it will be very helpful if someone can teach me where is the fault and better way to fix it.

output:

input file name: input.txt.txt
student01       1       95.000000
student03       3       90.000000
student05       5       86.000000
student07       7       83.000000
student09       9       98.000000
student10       10      93.000000
student08       8       92.000000
student06       6       96.000000
student04       4       93.000000
student02       2       88.000000

ID=2
Name=student02
Marks=88.000000

ID=11552328
â–‘ame=P
Marks=13640062821560266000000000000000000.000000


Process returned -1073741819 (0xC0000005)   execution time : 6.737 s
Press any key to continue.
c
linked-list
asked on Stack Overflow Dec 31, 2019 by ProgrammerNoob

2 Answers

1

It looks like it printed the last line and next line(which is garbage).I think the problem is in the read function?

This is correct.

And here is why:

    while(fscanf(fp, "%s %d %f", list->name, &list->id, &list->marks) == 3)
    {
        printf("%s \t%d \t%f\n", list->name,list->id,list->marks);//check
        list->next=(node*)malloc(sizeof(node));//create next node
    }

In this loop you always read all the data in the first element of your list. You allocate some new node but you never assign anything to it. You even do not terminate the list by assigning NULL to list->next->next.

This causes various problems:

  • You allocate memory for each iteration but only the last one is accessible via list->next. All other memory blocks are lost.
  • You have an unterminated list which causes undefined behaviour when you try to iterate over all nodes during printing.
  • You read all data into 1 location meaning you've lost all but the last line from your file.

To fix this you need to change it like this:

    while(fscanf(fp, "%s %d %f", list->name, &list->id, &list->marks) == 3)
    {
        printf("%s \t%d \t%f\n", list->name,list->id,list->marks);//check
        list->next=(node*)malloc(sizeof(node));//create next node
        if (list->next != NULL)
        {
            list = list->next; // Advance to new node
            list->next = NULL; // Terminate list at this node (until a new one is added)
        }
        else
        {
          // error handling
          exit(1);
        }
    }

Besides that:

  • You are obviously not using an embedded system where main might have void return type. For all hosted environments it has to return int.
  • Why do you define standard macro NULL? You should included the C standard header that defined it. Besides that NULLis a pointer value. Why do you use integer type?
answered on Stack Overflow Dec 31, 2019 by Gerhardh • edited Dec 31, 2019 by Gerhardh
0

The following proposed code:

  1. cleanly compiles
  2. performs most error checking
  3. actually produce a linked list of struct student
  4. documents why each header file is included
  5. eliminates most of the 'magic' numbers
  6. initializes the next field properly
  7. fails to pass the allocated memory areas to free()
  8. properly passes the address of head to the read() function so when the list is empty, the contents of head can be updated with the pointer to the first node
  9. properly initializes the head pointer to NULL
  10. properly links each new node to the prior node

and now, the proposed code:

#include <stdlib.h>   // FILE, fopen, fclose, scanf(), fscanf(), printf()
#include <stdio.h>    // malloc(), free(), exit(), EXIT_FAILURE
#include <string.h>   // memcpy()

#define MAX_NAME_LEN 20
#define MAX_FILENAME_LEN 30

struct student
{
    int id;
    char name[ MAX_NAME_LEN ];
    float marks;
    struct student *next;
};
typedef struct student node;


void read_echo_records  (node **p);
void create(node *p);
int  count (node *p);
void print (node *p);


int main( void )
{
    node *head = NULL;

    read_echo_records( &head);
    printf("\n");
    print(head);
}


void read_echo_records(node **list)
{
    FILE *fp;
    char filename[ MAX_FILENAME_LEN ];
    printf("input file name: ");
    scanf("%29s",filename);

    fp = fopen( filename, "r" );
    if( fp == NULL )
    {
        perror( "fopen failed" );
        exit( EXIT_FAILURE );
    }

    node *current = *list;
    node *prior   = *list;
    node workspace;
    workspace.next = NULL;

    while(fscanf(fp, "%s %d %f", workspace.name, &workspace.id, &workspace.marks) == 3)
    {
        printf("%s \t%d \t%f\n", workspace.name, workspace.id, workspace.marks);//check

        if( ! (current = malloc(sizeof(node))) )//create next node
        {
            perror( "malloc failed" );
            fclose( fp );
            exit( EXIT_FAILURE );
        }

        memcpy( current, &workspace, sizeof( node ) );
        *prior->next = current;
        prior = current;
    }
    fclose(fp);
    return;
}


void print(node*list)
{
    node *current = list;
    if( ! current )
    {
        printf( "%s\n", "list is empty" );
        return;
    }

    while( current->next )
    {
       printf( "ID=%d\n", current->id );
       printf( "Name=%s\n", current->name );
       printf( "Marks=%f\n", current->marks );
       printf( "\n");
       current = current->next;
    } 
}
answered on Stack Overflow Jan 1, 2020 by user3629249

User contributions licensed under CC BY-SA 3.0