• Najnowsze pytania
  • Bez odpowiedzi
  • Zadaj pytanie
  • Kategorie
  • Tagi
  • Zdobyte punkty
  • Ekipa ninja
  • IRC
  • FAQ
  • Regulamin
  • Książki warte uwagi

C - zapisanie danych z pliku do listy jednokierunkowej - pojawiające się losowe dane, nagłe kończenie pracy programu

0 głosów
82 wizyt
pytanie zadane 8 czerwca w C i C++ przez Sax Nowicjusz (150 p.)
edycja 8 czerwca przez Sax

Cześć,

piszę program, który ma między innymi pobrać dane z pliku. txt i zapisać je do listy jednokierunkowej. W 3/4 przypadków działa to bez zarzutu jednak czasem program kończy się błędem zgłaszając wyjście poza zakresy pamięci(1) lub wpisuje w listę losowe dane(2).

1: 
2: (w performer powinno być Fisz).

 

Niżej umieszczam kod. Przepraszam za bałagan, ale program jest jeszcze "rozgrzebany" i inne funkcjonalności (nie te związane z wczytaniem pliku) są zaimplementowane na pół gwizdka.

KOD:

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

struct Songs {
    char *label;
    char *name;
    char *performer;
    int time;
    char *localization;

    struct Songs *next;
};

struct Songs *head;

void help();

void insert(char *lbl, char *name, char *performer, int time, char *localization);

void display();

void read_file(char *file_name);

int parse_to_time(char *time);

void sort_by_time();

void sort_by_name();

void swap(struct Songs *a, struct Songs *b);

size_t getline(char **lineptr, size_t *n, FILE *stream);

int main(int argc, char **argv) {

    read_file("songs.txt");

    sort_by_name();

    display();

    free(head);

    return 0;
}

void help() {
    printf("-i input file with music\n");
    printf("-p playlist file\n");
    printf("-c time sort\n");
    printf("-w performer sort\n");
    printf("-t title sort\n");

    exit(1);
}

void insert(char *lbl, char *name, char *performer, int time, char *localization) {
    struct Songs *ptr, *temp;
    ptr = (struct Songs *) malloc(sizeof(struct Songs));
    if (ptr == NULL) printf("OVERFLOW\n");
    else {
        ptr->label = lbl;
        ptr->name = name;
        ptr->performer = performer;
        ptr->time = time;
        ptr->localization = localization;

        if (head == NULL) {
            ptr->next = NULL;
            head = ptr;
        } else {
            temp = head;

            while (temp->next != NULL) {
                temp = temp->next;
            }

            temp->next = ptr;
            ptr->next = NULL;
        }
    }
}

void display() {
    struct Songs *ptr;
    ptr = head;
    if (ptr == NULL) printf("Nothing to print\n");
    else {
        while (ptr != NULL) {
            printf("Label:\t\t%s\n"
                   "Name:\t\t%s\n"
                   "Performer:\t%s\n"
                   "Time:\t\t%d\n"
                   "Localization:\t%s\n\n", ptr->label, ptr->name, ptr->performer, ptr->time, ptr->localization);
            ptr = ptr->next;
        }
    }
}

void read_file(char *file_name) {
    FILE *fp;
    char *line = NULL;
    size_t len = 0;
    size_t read;

    int line_counter = 0;
    char *element[5];

    fp = fopen(file_name, "r");
    if (fp == NULL) printf("Empty file!");

    while ((read = getline(&line, &len, fp)) != EOF) {
        //if line in file is empty skip loop step
        if (strcmp(line, "\n") == 0) continue;

        //delete line break
        line[strcspn(line, "\n")] = 0;

        //allocate memory to string, copy line to array of strings
        element[line_counter] = (char *) malloc(sizeof(line));
        strcpy(element[line_counter], line);
        line = NULL;
        line_counter++;

        //if collected five values, insert it to a list
        if (line_counter > 4) {
            line_counter = 0;
            insert(element[0], element[1], element[2], parse_to_time(element[3]), element[4]);
        }
    }

    printf("\n\n");
    fclose(fp);
    if (line) free(line);
}

int parse_to_time(char *time) {

    int minutes, seconds;
    char *seconds_str;

    //takes seconds only
    strncpy(seconds_str, time + (strlen(time) - 2), 2);
    //take minutes only
    strtok(time, ":");

    minutes = atoi(time) * 60;
    seconds = atoi(seconds_str);

    return minutes + seconds;
}

void sort_by_time() {
    int swapped, i;
    struct Songs *ptr1;
    struct Songs *lptr = NULL;

    /* Checking for empty list */
    if (head == NULL)
        return;

    do {
        swapped = 0;
        ptr1 = head;

        while (ptr1->next != lptr) {
            if (ptr1->time > ptr1->next->time) {
                swap(ptr1, ptr1->next);
                swapped = 1;
            }
            ptr1 = ptr1->next;
        }
        lptr = ptr1;
    } while (swapped);
}

void sort_by_name() {
    int swapped;
    struct Songs *ptr;
    struct Songs *lptr = NULL;

    /* Checking for empty list */
    if (head == NULL)
        return;

    do {
        swapped = 0;
        ptr = head;

        while (ptr->next != lptr) {
            if (ptr->name[0] > ptr->next->name[0]) {
                swap(ptr, ptr->next);
                swapped = 1;
            }
            ptr = ptr->next;
        }
        lptr = ptr;
    } while (swapped);
}

void swap(struct Songs *a, struct Songs *b) {
    char *temp_lbl = a->label;
    a->label = b->label;
    b->label = temp_lbl;

    char *temp_name = a->name;
    a->name = b->name;
    b->name = temp_name;

    char *temp_perf = a->performer;
    a->performer = b->performer;
    b->performer = temp_perf;

    int temp_time = a->time;
    a->time = b->time;
    b->time = temp_time;

    char *temp_localization = a->localization;
    a->localization = b->localization;
    b->localization = temp_localization;
}

size_t getline(char **lineptr, size_t *n, FILE *stream) {
    char *bufptr = NULL;
    char *p = bufptr;
    size_t size;
    int c;

    if (lineptr == NULL) {
        return -1;
    }
    if (stream == NULL) {
        return -1;
    }
    if (n == NULL) {
        return -1;
    }
    bufptr = *lineptr;
    size = *n;

    c = fgetc(stream);
    if (c == EOF) {
        return -1;
    }
    if (bufptr == NULL) {
        bufptr = malloc(128);
        if (bufptr == NULL) {
            return -1;
        }
        size = 128;
    }
    p = bufptr;
    while (c != EOF) {
        if ((p - bufptr) > (size - 1)) {
            size = size + 128;
            bufptr = realloc(bufptr, size);
            if (bufptr == NULL) {
                return -1;
            }
        }
        *p++ = c;
        if (c == '\n') {
            break;
        }
        c = fgetc(stream);
    }

    *p++ = '\0';
    *lineptr = bufptr;
    *n = size;

    return p - bufptr - 1;
}

Plik z danymi:

Met1
Master of Puppets
Metalica
8:36
c:/muzyka/mmofp.mp3

Dod1
Nie daj sie
Doda
3:01
c:/muzyka/pop/doda1.mp3

Tac1
4 AM in Girona
Taco Hemingway
3:09
c:/muzyka/4am.mp3

Fis1
Czerowna sukienka
Fisz
5:35
c:/muzyka/idk/czer.mp3

Sub1
Cradles
Sub Urban
3:30
c:/muzyka/idk/sub_urban_cradles.mp3

Jai1
Dynabeat
Jain
2:53
c:/muzyka/idk/dynabeat.mp3

Byłbym wdzięczny za pomoc, na ten moment nie widzę co jest nie tak.

UPDATE: widzę że program spada z rowerka kiedy ma w tekstowym więcej niż 4 piosenki ¯\_(ツ)_/¯.

2 odpowiedzi

+1 głos
odpowiedź 9 czerwca przez j23 Mędrzec (162,820 p.)
wybrane 9 czerwca przez Sax
 
Najlepsza

Cieknie Ci z tego kodu, ale do rzeczy:

char *line = NULL;
...

element[line_counter] = (char *) malloc(sizeof(line));

sizeof(line) zawsze zwróci 4 lub 8 niezależnie od długości łańcucha znakowego.


  • Linia 44: to nie czyści listy, jedynie usuwa pierwszy element.
  • Linia 124: to przypisanie jest (chyba) zbędne. W każdym razie powoduje wyciek.
  • Linia 130: następny wyciek: parse_to_time(element[3]) pamięć z element[3] powinieneś zwolnić.

Na razie tyle.


getline można nieco uprościć:

size_t getline(char **lineptr, FILE *stream) 
{
    int c;
 
    if (!lineptr || !stream) { return -1;  }
    
    char* bufptr = *lineptr;
    size_t size = 0;
    size_t n = 0;
 
    while ((c = fgetc(stream)) != EOF && c != '\n') {
        if(n == size) {
            size += 128;
            bufptr = realloc(bufptr, size + 1);
            if(!bufptr) return -1;
        }
        bufptr[n++] = c;
        bufptr[n] = 0;
    }

    *lineptr = bufptr;
    return c == EOF && n == 0 ? -1 : n;
}

 

komentarz 9 czerwca przez Sax Nowicjusz (150 p.)
Jak w takim razie ugryźć ten kawałek kody, który wszystko psuje? Bo jakiś rozmiar muszę temu stringowi nadać, a ten zawsze będzie trochę inny, bo i co innego z pliku wejdzie.
Przepraszam, nie chcę być nachalny. Po prostu C to zupełnie nie moja bajka i nie wiem jak się za to zabrać.
1
komentarz 9 czerwca przez j23 Mędrzec (162,820 p.)
edycja 10 czerwca przez j23

Z moją wersją funkcji getline możesz czytać tak:

    while ((read = getline(&line, fp)) != -1) {

        int i = 0;
        while(i < read && strchr(" \t\r", line[i]) != 0) ++i;
        if(i == read) continue;

        element[line_counter++] = line;
        line = NULL;

        if (line_counter > 4) {
            line_counter = 0;
            insert(element[0], element[1], element[2], parse_to_time(element[3]), element[4]);
            free(element[3]);
        }
    }

Oczywiście popraw błąd w parse_to_time, o którym wspomniał @tangarr.

komentarz 9 czerwca przez Sax Nowicjusz (150 p.)
Dzięki wielkie
1
komentarz 10 czerwca przez j23 Mędrzec (162,820 p.)

Warunek z strcspn był bez sensu, więc go zmieniłem.

0 głosów
odpowiedź 9 czerwca przez tangarr VIP (135,700 p.)
W funkcji parse_to_time zapisujesz dane do niezainicjalizowanego wskaźnika. Zamień go na tablicę o statycznej długości.
Dodatkowo może się zdarzyć, że pusta linia oddzielająca utwory nie będzie pusta (np. zawiera spacje).
W kodzie jest przynajmniej 5 wycieków pamięci.

Podobne pytania

0 głosów
1 odpowiedź 52 wizyt
pytanie zadane 8 maja 2020 w C i C++ przez Dyali56 Nowicjusz (150 p.)
0 głosów
1 odpowiedź 198 wizyt
pytanie zadane 19 stycznia 2019 w C i C++ przez dzuulie Nowicjusz (120 p.)
+1 głos
1 odpowiedź 132 wizyt
Porady nie od parady
Odznacz odpowiedź zieloną fajką, jeśli uważasz, że jest ona najlepsza ze wszystkich i umożliwiła ci rozwiązanie problemu.Najlepsza odpowiedź

83,605 zapytań

132,255 odpowiedzi

291,743 komentarzy

55,205 pasjonatów

Motyw:

Akcja Pajacyk

Pajacyk od wielu lat dożywia dzieci. Pomóż klikając w zielony brzuszek na stronie. Dziękujemy! ♡

Oto dwie polecane książki warte uwagi. Pełną listę znajdziesz tutaj.

...