在C语言中将结构体成员保存到.txt文件时出现意外结果

Question

我在尝试将程序中的信息保存到.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()？），那是因为我们老师对我们的作业设置了一些非常奇怪的规则。请随意抨击我的代码。

- lemonslayer

---

首先明显的问题是：glist.list显然应该是一个指向grocery对象的指针数组，但你只分配了一个内存空间。其次，addGrocery假设存在一个grocery项目，但第一次调用它时，显然还没有创建任何grocery项目，所以你的scanf输入会进入随机的内存地址。事实上，我没有看到你在任何地方分配grocery项目的内存空间。 - Lee Daniel Crocker

@LeeDanielCrocker 我们的老师让我们创建一个指针，为其分配内存（类似于数组）。当您向列表中添加新项目时，您将沿途扩展所需的内存（realloc()）。我不明白addGrocery()如何假定内存中已经有一个项目？我错过了什么吗？ - lemonslayer

像往常一样，T*p=[m/c/re]alloc(sizeof T*)是错误的。 - EOF

指针列表是需要内存的一件事情。但是你还需要为它们所指向的每个结构体分配内存。另一种选择是将GList.list声明为零长度数组，而不是指针，然后以整个结构单元进行分配。 - Lee Daniel Crocker

是的，你很好地分配了 n 个指针。现在它们指向什么？ - Lee Daniel Crocker

Answer 1

请问，C语言中使用0-based还是1-based数组索引？

当你调用addGrocery函数时，你需要传递glist的地址。

addGrocery(&glist);

当您第一次调用addGrocery时，glist的初始值是什么？在添加第一个项目之前，此列表包含多少个项目？这是一个“列表”还是一个“数组”？

以下是主函数的前几行（回答了这个问题）：

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 声明为指针，并在主函数中初始化。您是否已经分配了一些项目的数组，还是想要一个项目列表？结构体 Grocery 类型没有指针，因此您可能不想要一个 "list"，而是一个 "array"（命名很重要）。

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

因为您的addGrocery函数使用了数组索引，所以我们需要一个杂货项数组，但是您创建了多少个？而且您要操作哪一个呢？

（这些问题应该会指引您朝正确的方向思考）

- ChuckCottrill

---

这些问题确实指引了我正确的方向！非常感谢您的帮助，您的帖子让我有很多思考，但现在我对这个问题有了更好的理解:D - lemonslayer

Answer 2

首先，我相信你的老师会多次告诉你不要使用魔数：

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手册（作为软件开发人员的一部分）。事实上，即使从粗略的浏览中，你也可能不会意识到自己做错了什么。

事实上，作为软件开发人员，我们不仅必须仔细阅读手册、错误消息和其他人编写的代码（这些代码可能不能很好地反映出质量较差的注释）。

检查 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可能不会在覆盖旧指针之前释放它（从而导致内存泄漏）。因此，我在这里使用了一个临时变量。同样，在这里你也错过了另一个星号。

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;

---

顺便问一句，你听说过 valgrind 吗？...

- autistic

---

不，我没有听说过valgrind，但是我真的非常感激你抽出时间仔细检查所有内容！我学到了很多关于我自己经常犯的错误，其中很多我都知道，但在尝试实现时却总是做错，我需要更多地学习:D 非常感谢你！ - lemonslayer

现在你已经听说过 valgrind 了... 对于其他操作系统，你可以通过谷歌搜索 "valgrind 替代品" 来找到替代品。问题是，你是否会利用谷歌的力量来寻找知识，这将帮助你在几分钟内发现和修复此类问题，而不是浪费数小时查找基本的拼写错误呢？ - autistic