how to stop cycle that creates a linked list to create one more empty link?

Viewed 45
struct data {
  int date;
  int temperature;
  char day[11];
  struct data *next;
}; 
typedef struct data Data;

int from_file_to_list(File *fp, Data **list) {
  Data *p;
  int cnt = 0;
  *list = p;
  p = malloc(sizeof(Data));
  while (fscanf(fp, "%d" "%s" "%d", &(p->date), p->day, &(p->temperature)) == 3) {
    p->next =  malloc(sizeof(Data));
    p = p->next;
    cnt++;
  }
  return cnt;
}

I have this function that creates a linked list with the data of a text file and it returns the number of links in the list and it works fine, but it allocates memory for an additional empty link at the end, is there any way to stop that? Because if i would have a function that adds a link in the end there could be problems right?

2 Answers

For starters it seems there is a typo

int from_file_to_list(File *fp, Data **list) {
                      ^^^^

There should be

int from_file_to_list(FILE *fp, Data **list) {
                      ^^^^

I have this function that creates a linked list with the data of a text file and it returns the number of links in the list and it works fine,

You are mistaken. The function has undefined behavior. In this statement

int from_file_to_list(File *fp, Data **list) {
  Data *p;
  int cnt = 0;
  *list = p;
  //.. 

you assigned uninitialized pointer p to the pointer *list. That is after exiting the function the pointer list in main will have this indeterminate value.

The function can be defined for example at least the following way

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

//... 

size_t from_file_to_list( FILE *fp, Data **list ) 
{
    size_t cnt = 0;

    int date;
    int temperature;
    char day[11];
    
    while ( fscanf( fp, "%d" "%s" "%d", &date, day, &temperature ) == 3 && 
            ( *list = malloc( sizeof( Data ) ) ) != NULL ) 
    {
        ( *list )->date = date;
        ( *list )->temperature = temperature;
        strcpy( ( *list )->day, day );
        ( *list )->next = NULL;
        
        list = &( *list )->next;
        ++cnt;
    }
    
    return cnt;
}

Or even it will be better to free the list before creating a new list from a file

size_t from_file_to_list( FILE *fp, Data **list ) 
{
    while ( *list )
    {
        Data *tmp = *list;
        *list = ( *list )->next;
        free( tmp );
    }
    
    size_t cnt = 0;

    int date;
    int temperature;
    char day[11];
    
    while ( fscanf( fp, "%d" "%s" "%d", &date, day, &temperature ) == 3 && 
            ( *list = malloc( sizeof( Data ) ) ) != NULL ) 
    {
        ( *list )->date = date;
        ( *list )->temperature = temperature;
        strcpy( ( *list )->day, day );
        ( *list )->next = NULL;
        
        list = &( *list )->next;
        ++cnt;
    }
    
    return cnt;
}

If you will write a separate function that clears a list like

void clear( Data **list )
{
    while ( *list )
    {
        Data *tmp = *list;
        *list = ( *list )->next;
        free( tmp );
    }
}    

then the above function can be written like

size_t from_file_to_list( FILE *fp, Data **list ) 
{
    clear ( list );
    
    size_t cnt = 0;

    int date;
    int temperature;
    char day[11];
    
    while ( fscanf( fp, "%d" "%s" "%d", &date, day, &temperature ) == 3 && 
            ( *list = malloc( sizeof( Data ) ) ) != NULL ) 
    {
        ( *list )->date = date;
        ( *list )->temperature = temperature;
        strcpy( ( *list )->day, day );
        ( *list )->next = NULL;
        
        list = &( *list )->next;
        ++cnt;
    }
    
    return cnt;
}

You're setting *list = p; before p is set, so you have UB (undefined behavior).

With that fixed, your code would not handle an empty list properly.

Better to read in the values into a temp variable and do a malloc only if there is a valid data line.

Don't do malloc for the next record. On the last line of the file, you'd have an extra struct allocation.

Here's some refactored code. It is annotated. I've compiled it but not tested it:

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

struct data {
    int date;
    int temperature;
    char day[11];
    struct data *next;
};
typedef struct data Data;

int
from_file_to_list(FILE *fp, Data **list)
{
    Data temp;
    Data *prev;
    Data *cur;
    int cnt = 0;

    // make the list empty
    *list = NULL;

    prev = NULL;

    // loop on all lines/records in the file
    while (fscanf(fp, "%d" "%s" "%d",
        &temp.date, temp.day, &temp.temperature) == 3) {

        // allocate new record
        cur = malloc(sizeof(*cur));

        // copy in the data we just read
        *cur = temp;

        // append to non-empty list
        if (prev != NULL)
            prev->next = cur;

        // the first record
        else
            *list = cur;

        // remember the previous record
        prev = cur;

        // advance the count of number of records
        cnt++;
    }

    return cnt;
}
Related