Неочакван резултат при запазване на членове на структура в .txt файл в C

Имам няколко проблема, докато се опитвам да запазя информация от моята програма в .txt файл (или във всеки друг файл). Преглеждах кода си няколко пъти и не мога да намеря проблема. Първоначално си мислех, че може да има някаква форма на изтичане на памет (въпреки че не знам последствията от изтичане на памет, така че не мога да съм сигурен).

Искам да поясня, че това е училищна задача и в крайна сметка трябва да уча, така че не ми давайте отговора твърде лесно!

Поръчката ни е последна и най-голяма. Създаваме списък с хранителни стоки със структури. Проблемът започна, когато се опитвах да използвам членовете на структурата, за да запазя информация в .txt файл (за да ги заредя в програмата по-късно, ако желаете). Знам, че кодът вероятно е ужасен и дразнещ за гледане, но моля, изтърпете ме.

Това е моята функция "запазване". Това е много елементарно и доста ужасно.

void saveList(struct GList *grocery)
{
    char file[20];
    FILE *fp;

    printf("What do you want to save the list as? (Don't include file extension): ");
    scanf("%s", file);

    fp = fopen(strcat(file, ".txt"), "w");

    for (int i=0; i<grocery->Items; i++)
    {
        printf("%s %f %s\n", grocery->list[i].name, grocery->list[i].amount, grocery->list[i].unit);
        fprintf(fp, "%s %f %s\n", grocery->list[i].name, grocery->list[i].amount, grocery->list[i].unit);
    }
    fclose(fp);
}

Това е, което въвеждам в моята програма (когато добавям елемент):

Name of the item: Avocado
Unit: kg
Amount: 10

Това е, което се записва в моя .txt файл (не се показва, но първият ред винаги съдържа някакъв странен символ).

  10.000000 kg
milk 10.000000 litres

Същият проблем възниква през цялото време; първото име на елемент (напр. авокадо) се показва като някакъв странен символ.

Това е пълният ми код, проблемът може да се крие някъде тук.

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

struct Grocery {
    char name[20];
    char unit[20];
    float amount;
};

struct GList {
    size_t Items;
    struct Grocery *list;
};

int addGrocery();
void printList();
void hQuit();
void inputError();
void removeItem();
void changeItem();
void saveList();

int main()
{
    struct GList glist;
    glist.Items = 1;

    size_t menuChoice = 0;
    char cont = 'y';

    if((glist.list = malloc(sizeof(glist.list))) == NULL)
        return ENOMEM;

    puts("Welcome to your Grocery List Manager");

    do
    {
        printf("\n- - - - - - - - - - -\n[1] Add an item\n[2] Print grocery list\n[3] Remove a grocery\n[4] Edit a grocery\n[5] Save your list\n[6] Load a list\n[7] Quit\n\nPlease choose action: ");
        if(!scanf("%u", &menuChoice))
            return EIO;

        putchar('\n');

        switch(menuChoice)
        {
            case 1:
                addGrocery(&glist);
                break;
            case 2:
                printList(&glist);
                break;
            case 3:
                removeItem(&glist);
                break;
            case 4:
                changeItem(&glist);
                break;
            case 5:
                saveList(&glist);
                break;
            case 6:
                //Load shopping list
                break;
            case 7:
                hQuit(&glist);
                break;
            default:
                inputError();
                break;
        }
    } while (cont == 'y');

    //free(grocery);

    return 0;
}

int addGrocery(struct GList *grocery)
{
    printf("Name of the grocery: ");
    if(!scanf("%s", grocery->list[grocery->Items].name))
        return EIO;

    printf("Unit: ");
    if(!scanf("%s", grocery->list[grocery->Items].unit))
        return EIO;

    printf("Amount: ");
    if(!scanf("%f", &grocery->list[grocery->Items].amount))
        return EIO;

    printf("You have added %f %s of %s into your list!\n\n", grocery->list[grocery->Items].amount, grocery->list[grocery->Items].unit, grocery->list[grocery->Items].name);

    (grocery->Items)++;
    grocery->list = realloc(grocery->list, grocery->Items * sizeof(grocery->list));

    if(grocery->list == NULL)
        return ENOMEM;

    return 1;
}

void printList(struct GList *grocery)
{
    if ((grocery->Items - 1) > 0)
        printf("You have added %d item(s) into your list!\n", grocery->Items - 1);
    else
        printf("You have no items in your list!\n");

    for (int i=1; i<grocery->Items; i++)
    {
        printf("[%d] %-10s %.1f %s\n", i, grocery->list[i].name, grocery->list[i].amount, grocery->list[i].unit);
    }
    putchar('\n');
}

void removeItem(struct GList *grocery)
{
    size_t index = 0;
    printf("Which item would you wish to remove from the list? ");
    scanf("%u", &index);
    printf("\nYou have removed %s from your grocery list!", grocery->list[index].name);
    for (int i=(int)index; i < grocery->Items; i++)
        grocery->list[i] = grocery->list[i+1];

    (grocery->Items)--;
}

void changeItem(struct GList *grocery)
{
    size_t index = 0;
    printf("Which item would you like to edit the amount of? ");
    scanf("%d", &index);
    printf("\nCurrent amount: %.1f %s\nEnter new amount: ", grocery->list[index].amount, grocery->list[index].unit);
    scanf("%f", &grocery->list[index].amount);
    printf("\nYou changed the amount to %.1f!\n", grocery->list[index].amount);
}

void hQuit(struct GList *grocery)
{
    puts("*-*-* Thank you for using the Grocery List! *-*-*");
    free(grocery->list);
    exit(0);
}

void inputError(struct GList *grocery)
{
    puts("No such option. Please try again!\n");
}

void saveList(struct GList *grocery)
{
    char file[20];
    FILE *fp;
    printf("What do you want to save the list as? (Don't include file extension): ");
    scanf("%s", file);
    fp = fopen(strcat(file, ".txt"), "w");
    for (int i=0; i<grocery->Items; i++)
    {
        printf("%s %f %s\n", grocery->list[i].name, grocery->list[i].amount, grocery->list[i].unit);
        fprintf(fp, "%s %f %s\n", grocery->list[i].name, grocery->list[i].amount, grocery->list[i].unit);
    }

    fclose(fp);
}

Ако кодът ми изглежда много странно на някои места (защо имате void inputError()?), това е защото нашият учител задава някои много странни правила за нашите задачи.

Моля, не се колебайте да разбиете моя код.


person lemonslayer    schedule 11.01.2018    source източник
comment
Първи очевиден проблем: glist.list очевидно е предназначен да бъде масив от указатели към хранителни обекти, но вие разпределяте памет само за един. Второ: addGrocery предполага, че съществува хранителен артикул, но първият път, когато го извикате, очевидно не е създаден никакъв хранителен артикул, така че сканирането отива в произволна памет. Всъщност не виждам да разпределяте памет за хранителни стоки навсякъде.   -  person Lee Daniel Crocker    schedule 11.01.2018
comment
@LeeDanielCrocker Нашият учител: Създайте указател, за който ще разпределите памет (както работи масив). Когато добавите нов елемент към списъка, вие ще разширите необходимата памет по пътя (realloc()). Не разбирам как addGrocery() приема, че вече има артикул в паметта? Изпускам ли нещо?   -  person lemonslayer    schedule 11.01.2018
comment
Както винаги, T*p=[m/c/re]alloc(sizeof T*) греши.   -  person EOF    schedule 11.01.2018
comment
Списъкът с указатели е едно нещо, за което се нуждаете от памет. Но вие също се нуждаете от памет за всяка от структурите, към които сочат. Алтернативата е да декларирате GList.list като масив с нулева дължина вместо указател и след това да разпределите в цели структурни единици.   -  person Lee Daniel Crocker    schedule 11.01.2018
comment
@LeeDanielCrocker Така че едно от заданията не беше използването на масиви, а използването на указатели заедно с malloc() и realloc(). Мислех, че malloc(n * sizeof(list)) е основно същото като list[n], с изключение на това, че е динамична памет.   -  person lemonslayer    schedule 11.01.2018
comment
Да, разпределяте n указатели съвсем добре. Сега какво сочат?   -  person Lee Daniel Crocker    schedule 11.01.2018
comment
Какво беше въведено, когато scanf("%s", file); се изпълни?   -  person chux - Reinstate Monica    schedule 11.01.2018
comment
@chux test1, test2 и т.н., името на файла, който исках да запазя.   -  person lemonslayer    schedule 11.01.2018


Отговори (2)


Запитайте се „С използва ли базирано на 0 или 1 индексиране на масиви“?

Когато извикате addGrocery, предавате адреса на glist,

addGrocery(&glist);

Каква е първата/началната стойност на glist, когато извикате addGrocery за първи път? Колко елемента съдържа този списък, преди да добавите първия елемент? Това "списък" ли е или "масив"?

Ето първите няколко реда от вашата основна функция (която отговаря на този въпрос),

int main()
{
    struct GList glist;
    glist.Items = 1;

    if((glist.list = malloc(sizeof(glist.list))) == NULL)
        return ENOMEM;

Помислете за дефиниране на функция (конструктор), за да създадете първоначалния (празен) списък. И функция за добавяне на елемент към списъка.

Вашата функция addGrocery обединява въвеждането на данни и добавянето на данните към списъка. Помислете за функция, която просто събира входни данни, след което извиква функцията, за да добави данните към списъка.

int addGrocery(struct GList *grocery)
{
    printf("Name of the grocery: ");
    //what is the value of grocery-Items the first time this is called?
    if(!scanf("%s", grocery->list[grocery->Items].name))
        return EIO;

    //Consider something that creates a grocery list item (does malloc)
    //then appends that list item to the list

    //then this check would not be needed (well, it would change)
    if(grocery->list == NULL)
        return ENOMEM;

Съвет: Добавяте ли към първия елемент от списъка?

Но има още по-голям проблем. Използвате ли масив или списък за съхраняване на структурни хранителни продукти? Декларираш list като указател и го инициализираш в main. Задали ли сте масив от някакъв брой елементи или искате списък с елементи? Типът struct Grocery няма указатели, така че вероятно не искате "списък", а по-скоро "масив" (именуването е важно).

struct GList {
    size_t Items;
    struct Grocery *list;
};

Тъй като вашата функция addGrocery използва индексиране на масиви, приемете, че искате масив от хранителни продукти, но колко сте създали? И към коя се обръщате?

(тези въпроси трябва да ви насочат в правилната посока)

person ChuckCottrill    schedule 11.01.2018
comment
И тези въпроси наистина ме насочиха в правилната посока! Благодаря много за помощта, публикацията ви ме накара да помисля много, но сега разбирам по-добре проблема :D - person lemonslayer; 11.01.2018

За начало съм сигурен, че вашият учител ще ви каже още много пъти да не използвате магически числа:

char file[PATH_MAX];

Вероятно бихте искали да избегнете препълването на този буфер в името на бъдещата изчислителна рационалност на вашите програми:

if (snprintf(NULL, 0, "%s.txt", file) >= PATH_MAX - 1) {
    fputs("Filename too long!", stderr);
    exit(EXIT_FAILURE);
}

if (scanf("%s", grocery->list[grocery->Items].name) == 1)

Няма да знаете, че използвате scanf неправилно, докато не прочетете ръководството за scanf< /a> (което е част от работата ни като разработчици на софтуер). Всъщност може дори да не изглежда очевидно от бегъл поглед, че правите нещо нередно.

Наистина, като разработчици на софтуер ние не само трябва внимателно да четем ръководства, съобщения за грешка, код написан от други хора (което може да не отразява добре коментарите с лошо качество).

Проверката дали scanf връща 0 е добър начин да се определи дали 0 елемента са били прочетени, но не е добър начин да се определи дали е възникнала EOF или някаква друга грешка при достъпа до файл.

Можете ли да разберете защо (правилно) сравних с 1 вместо това? Ако искате да прочетете две стойности от stdin, като използвате едно сравнение с scanf, с коя числова стойност трябва да сравните върнатата стойност?


void *temp = realloc(grocery->list, grocery->Items * sizeof *grocery->list);
if (temp == NULL)
    return ENOMEM;
grocery->list = temp;

Няма да знаете, че използвате realloc incorre... прочетете ръководството за realloc дядо ... дядо ... и т.н. и така нататък

Има още една модификация, която направих тук: пропуснахте звездичка! Оо! Вижте дали можете да я забележите :)

Наистина, като разработчици на софтуер ние не само трябва внимателно да четем ръководства, съобщения за грешка, код написан от други хора (което може да не отразява добре коментарите с лошо качество).

В резултат на това трябва да направим някои извадки от ръководството, за да определим кога realloc може да не free стария указател, преди да го презапишете (по този начин причинявайки изтичане на памет).

Можете ли да разберете защо използвах променлива temporary там (както трябва)?

По същия начин сте пропуснали друга звездичка тук:

if((glist.list = malloc(sizeof glist.list[0])) == NULL)

Съжалявам, не можах да не го направя малко по-очевидно... Трябва да си направите умствена бележка да следвате тези модели:

pointer = malloc(count * sizeof *pointer);               // -- the `malloc` pattern; pointer could be NULL, so needs checking
void *temp = realloc(pointer, count * sizeof *pointer);  // -- the `realloc` pattern; temp could be NULL (in which case you need to deal with `pointer`)

Запомнете тези два модела и не би трябвало да откриете, че правите отново тези грешки.

Докато сме на темата за списъците, празните съдържат 0 елемента, нали?

glist.Items = 0;

P.S. Чували ли сте някога за valgrind?...

person autistic    schedule 11.01.2018
comment
Не, не съм чувал за valgrind, но наистина оценявам, че отделихте време да преминете през всичко по този начин! Научих много за собствените си грешки, които съм склонен да правя, много от тези неща знам, но по някаква причина ги правя погрешно, когато се опитвам да го приложа, трябва да проуча повече :D Благодаря ви много! - person lemonslayer; 11.01.2018
comment
Е, сега чухте за valgrind... има алтернативи за други операционни системи, които можете да намерите, като потърсите в гугъл алтернативи на valgrind. Въпросът е дали ще използвате силата на Google, за да намерите знанието, което е силата, която ще ви помогне да откриете и отстраните проблеми като този за минути, вместо да губите часове в търсене на основни правописни грешки. ..? - person autistic; 11.01.2018