C file logs with list

1

I'm doing a file practice with a Person list that has a name and age. My problem is that when I run for the first time I can register and read in the file, but when I close and run again and I want to read the records again, the program crashes, that is, the .exe has stopped working.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <conio.h>
#define ARCHIVE_PERSONS "persons.bin"

typedef struct Person
{
    int age;
    char *name;
    struct Person *next;
}Person;

void pause(){
    printf("Presiona cualquier tecla para continuar\n");
    getch();
}

void savePerson(Person *newPerson){
    FILE * file;
    file=fopen(ARCHIVE_PERSONS,"ab");
    if(!file){
        printf("No se ha podido abrir el archivo\n");
        pause();
        exit(1);
    }
    else
        fwrite(
         newPerson,sizeof(Person),1,file);
    fclose(file);
}

Person * createPerson(int age, char * name){
    Person *newPerson;
    newPerson=(Person*)malloc(sizeof(Person));
    if(!newPerson){
        printf("No se ha podido reserver memoria para hacer el registro");
        exit(1);
    }
    else{
        newPerson->age=age;
        newPerson->name=name;
        newPerson->next=NULL;
        savePerson(newPerson);
    }
    return newPerson;
}

Person * insertPerson(Person *p, int age, char *name){
    Person *newPerson, *pAux;
    newPerson=createPerson(age,name);
    if(newPerson){
        if(!p)
            p=newPerson;
        else{
            pAux=p;
            while(pAux->next)
                pAux=pAux->next;
            pAux->next=newPerson;
        }
    }
    return p;
}

Person * registerPerson(Person *p){
    int age;
    char *name;
    char aux[50];
    system("cls");

    printf("Ingrese el nombre de la persona\n");
    fflush(stdin);
    gets(aux);
    name=(char*)malloc((sizeof(char)*strlen(aux))+1);
    if(!name){
        printf("no se pudo crear el nombre\n");
        exit(1);
    }
    else{
        strcpy(name,aux);
    }
    printf("Ingrese la edad de la persona\n");
    scanf("%i", &age);

    p=insertPerson(p,age, name);
}

void showPerson (){
    FILE * file;
    file=fopen(ARCHIVE_PERSONS,"rb");
    Person *newPerson;
    newPerson=(Person*)malloc(sizeof(Person));

    if(!file){
        printf("El archivo esta vacio\n");
        pause();
    }

    if(!newPerson){
        printf("No se puede leer archivo\n");
        pause();
    }else{
        system("cls");
        printf("%-10s %-10s\n","Nombre","Edad");
        while(fread(newPerson,sizeof(Person),1,file))
            printf("%-10s %-10i\n",newPerson->name,newPerson->age);
    }
    fclose(file);
}

void menu(){
    int op;
    Person *p;
    p=NULL;
    do{
        system("cls");
        printf("[1]Ingresar persona\n");
        printf("[2]Mostrar personas\n");
        printf("[3]Salir\n");
        scanf("%i",&op);

        switch(op){
            case 1: p=registerPerson(p); break;
            case 2: showPerson(); break;
            case 3: exit(1); break;
            default: printf("Opcion incorrecta\n");break;
        }
        pause();
    }while(op!=3);
}

int main(){
    menu();
    pause();
}
    
asked by Mauricio Brito 06.07.2016 в 22:16
source

2 answers

3

Problem

Your problem is in the writing in file:

fwrite(
 newPerson,sizeof(Person),1,file);

You are writing directly into the file the person structure which does not directly contain all the data for which it is responsible. Let's see what it might look like in memory (assuming 32-bit pointers):

0: |     age   |    name   |    next   |
   |00|01|02|03|04|05|06|07|08|09|10|11|
   |         35| 0x00000001| 0x00000002|

1: |name de la estructura en 0|
   |00|01|02|03|04|05|06|07|08|
   | M| a| u| r| i| c| i| o|
int age;
char *name;
struct Person *next;
| 2: | age | name | next | |00|01|02|03|04|05|06|07|08|09|10|11| | 21| 0x00000003| 0x00000004|

The newPerson pointer you type in the file contains three values, two of which are pointers :

int namelen = strlen(newPerson->name);
fwrite(&newPerson->age, sizeof(newPerson->age), 1,       file);
fwrite(&namelen,        sizeof(namelen),        1,       file);
fwrite(newPerson->name, sizeof(char),           namelen, file);

That is to say: two of the values that Person manages, are memory addresses to a different zone than newPerson belongs to; when you write in the file you are writing the memory addresses that in that execution each of the instances of Person was using; when you recover this data from file you are recovering memory addresses from the previous execution and use them in the present execution, which surely causes your crash.

Solution

Do not write the structure Person directly, you must serialize it . To do this, first write the age, then the length of the name and to finish the name:

int namelen = 0, age = 0;
Person *newPerson =(Person*)malloc(sizeof(Person));

// Leer edad y longitud de nombre.
fread(&age,     sizeof(age),     1, file);
fread(&namelen, sizeof(namelen), 1, file);

// Reservar espacio para nombre (incluido caracter nulo).
newPerson->name =(char*)malloc((sizeof(char) * namelen) + 1);
memset(newPerson->name, 0, (sizeof(char) * namelen) + 1);

// Leer nombre.
fread(newPerson->name, sizeof(char), namelen, file);

// ... enlazar con siguiente Person ...

At the time of deserialize:

fwrite(
 newPerson,sizeof(Person),1,file);
    
answered by 07.07.2016 / 09:14
source
2
typedef struct Person
{
    int age;
    char *name;
    struct Person *next;
}Person;

Based on how you declare the previous structure in your code, there are several problems:

void showPerson (){
    FILE * file;
    file=fopen(ARCHIVE_PERSONS,"rb");
    Person *newPerson;
    newPerson=(Person*)malloc(sizeof(Person)); // (2)

    if(!file){ // (1)
        printf("El archivo esta vacio\n");
        pause();
    }

    if(!newPerson){
        printf("No se puede leer archivo\n");
        pause();
    }else{
        system("cls");
        printf("%-10s %-10s\n","Nombre","Edad");
        while(fread(newPerson,sizeof(Person),1,file)) // (4)
            printf("%-10s %-10i\n",newPerson->name,newPerson->age); // (3)
    }
    fclose(file);
}

1 If you have had problems opening the file, your current function will try to read the records anyway. The logical thing would be that when detecting the problem the program aborts the operation leaving the function:

    if(!file){
        printf("El archivo esta vacio\n");
        pause();
        return;
    }

On the other hand it does not make much sense to reserve dynamic memory for newPerson if we still do not know if the data in the file can be read. The normal thing is to make the reservation when we know that we can make the load:

    FILE * file;
    file=fopen(ARCHIVE_PERSONS,"rb");

    if(!file){
        printf("El archivo esta vacio\n");
        pause();
        return;
    }

    Person *newPerson = (Person*)malloc(sizeof(Person));

2 newPerson is a local variable that you initialize with malloc and that never is released, then every time this function is called leakage occurs. memory. Being a local variable, it is not necessary to use pointers, which allows you to not worry about memory management.

void showPerson (){
    FILE * file;
    file=fopen(ARCHIVE_PERSONS,"rb");
    Person newPerson;

    if(!file){
        printf("El archivo esta vacio\n");
        pause();
    }

    system("cls");
    printf("%-10s %-10s\n","Nombre","Edad");
    while(fread(&newPerson,sizeof(Person),1,file))
        printf("%-10s %-10i\n",newPerson.name,newPerson.age);

    fclose(file);
}

If you still insist on using dynamic memory remember to use free :

void showPerson (){
    FILE * file;
    file=fopen(ARCHIVE_PERSONS,"rb");

    // ...

    }else{
        Person *newPerson = (Person*)malloc(sizeof(Person));

        system("cls");
        printf("%-10s %-10s\n","Nombre","Edad");
        while(fread(newPerson,sizeof(Person),1,file))
            printf("%-10s %-10i\n",newPerson->name,newPerson->age);

        free(newPerson); // <<--- IMPORTANTE
    }
    fclose(file);
}

3 You are saving a string of at most 10 characters in new_person->name ... This would not be a problem if it were not because new_person->name does not point to a valid memory address. If the maximum size of the name is bounded, the simplest thing is to make the structure have a fixed size to store the name:

// Para almacenar hasta 10 caracteres
#define MAX_NAME_LENGTH 11

typedef struct Person
{
    int age;
    char name[MAX_NAME_LENGTH];
    struct Person *next;
}Person;

4 When reading from the file you are trying to retrieve the structure literally. Storing a raw structure in a file does not have to be problematic unless it has pointers. In your case, the pointer in question is next . When you dump the structure to the file that pointer will store the memory position it points to. Note that this position will not be valid when you try to recover the data in the file.

The clearest thing to solve this problem is to divide the structure in two: on the one hand a structure that stores the data and another for the management of the list:

typedef struct
{
  int age;
  char name[MAX_NAME_LENGTH];
} Person;

typedef struct node
{
  Person person;
  struct node* next;
} node;

fread(&node->person,sizeof(Person),1,file); // Ahora ya no se leen punteros
fwrite(&node->person,sizeof(Person),1,file); // Ya no se guardan punteros

And then already, as additional notes:

fflush(stdin);

fflush should only be used with output buffer, its use with input systems may cause indeterminate results.

a greeting.

    
answered by 07.07.2016 в 09:12