Неожиданный результат при сохранении членов структуры в файл .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)


Спросите себя: «Использует ли C индексацию массива на основе 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;

Подсказка: вы добавляете в первый элемент списка?

Но есть еще большая проблема. Вы используете массив или список для хранения элементов структуры Grocery? Вы объявляете 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 неправильно... прочитайте руководство 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