当释放内存时，将字符串数据插入链表会导致程序崩溃

Question

关于StackOverflow的第一篇文章(以测试人员为职业，业余时间非常糟糕的程序员)。为了公开起见，这是我正在努力完成的一项大学作业(据我所知，在这里提问并不被禁止)。

无论如何，我有一个程序从文本文件中读取一行，标记行数据，创建一个链表，然后将每个标记(2个字符串，1个浮点数，1个无符号)插入节点。一切正常，直到节点使用的内存被释放——整个程序崩溃。在调试之后，我似乎已经将问题与两个字符串复制操作隔离开来。它们看起来都是完全有效的，但是free()根本不喜欢它们。strncpy() -没有区别。尝试逐个字符复制字符串-没有区别。现在我很茫然……

下面的代码，如果有人想看一下(哦，更多的披露-几乎是一个完整的C n00b，所以是的，如果你看到下面的糟糕做法，那就是我…)

#include <assert.h>
#include <stdio.h>
#include <stdlib.h>
#include "ts.h"
int main(void)
{
    /* Variables */
    FILE *stream;
    char buf[BUFSIZ];
    /* Open stock file */         //TODO - make this dynamic, don't hard-code file name
    stream = fopen("stock.csv", "r");
    assert(stream);
    /* Using addStockNode */
    while (fgets(buf, BUFSIZ, stream))
    {
        addStockNode(buf);
    }
}
void addStockNode(char* stockLine)
{
    //Create linked list
    StockNodePtr head, new, current, previous, nextStock;
    unsigned listSize;
    char *stkTok1, *stkTok2, *stkTok3, *stkTok4;
    unsigned stkLevel;
    int i;
    float stkPrice;
    listSize = 0;
    head = NULL;
        /* Create new stock node */
        if ((new = malloc(sizeof(StockNodePtr))) == NULL)
        {
            fprintf(stderr,"nMemory allocation for node insertion failedn");
            fprintf(stderr,"Abortingn");
            exit(EXIT_FAILURE);
        }
        /* Tokenise data */
        stkTok1 = strtok(stockLine, ",");
        stkTok2 = strtok(NULL, ",");
        stkTok3 = strtok(NULL, ",");
        stkTok4 = strtok(NULL, ",");
        /* Search to find where in insert new list node */      //TODO - needs to be adapted to sort by stock DESCRIPTION
        current = head;
        previous = NULL;
        /* stockID */
     // strcpy(new->stockID, stkTok1);                 //falls over at free()
     // strncpy(new->stockID, stkTok1, STOCKID_LEN);   //falls over at free()
        for(i = 0; i < strlen(stkTok1); i++)           //still falls over at free()
        {
            new->stockID[i] = stkTok1[i];
        }
        /* description */
     // strcpy(new->description, stkTok2);              //falls over at free()
        /* unitPrice */
        stkPrice = strtof(stkTok3, NULL);
        new->unitPrice = stkPrice;
        /* StockLevel */
        stkLevel = strtol(stkTok4, NULL, 10);
        new->stockLevel = stkLevel;
        /*nextStock */
        new->nextStock = current;
        /* Increment listSize */
        listSize++;
        //TAKE OUT LATER - loadData can iterate through each line of the file */
        if(previous == NULL)            
        {
            head = new;
        }
        else
        {
            previous->nextStock = new;
        }
    /* Print node details */
    current = head;
    printf("%s,%s,%f,%in", current->stockID, current->description, current->unitPrice, current->stockLevel);       
    /* Deallocate memory used by node */
    current = head;
    while(current != NULL)
    {
        nextStock = current->nextStock;
        free(current);    //EXECUTE THIS, IT FALLS OVER (with strcpy lines uncommented)
        current = nextStock;
    }
    return EXIT_SUCCESS;
}*

为完整起见，下面是标准节点结构体…

typedef struct stockNode
{
   char stockID[STOCKID_LEN + 1];
   char description[DESCRIPTION_MAX + 1];
   float unitPrice;
   unsigned stockLevel;
   StockNodePtr nextStock;
} StockNodeType;

如果有人能指出我错在哪里，我会很感激的!

编辑-这里是股票节点常数…

#define STOCKID_LEN 5
#define DESCRIPTION_MAX 40
#define PRICE_COLWIDTH 7
#define STOCKLEVEL_COLWIDTH 3
#define STOCKLEVEL_MAX 100

哦，还有正在添加的股票数据(不是说它有什么问题)…

S0001,Slazenger Classic Racquet,150.00,5
S0002,Slazenger Lite Racquet,98.00,3
S0003,Wilson Tournament Gold Balls,14.95,20
S0004,Dunlop Grand Prix Balls,10.95,25
S0005,Luft Nemesis Racquet,125.00,1
S0006,Wilson Tournament Balls,12.95,12

Answer 1

主要缺陷

你的根本问题是分配不足，特别是这个:

if ((new = malloc(sizeof(StockNodePtr))) == NULL)

这意味着你分配了足够大的空间来容纳一个指针;不是股票节点。

试试这个:

if ((new = malloc(sizeof(*new))) == NULL)

注意:我没有完全阅读这部分内容，但它本身是相当大的，因为每个节点分配的内存永远不会超过指针的大小。如果还有其他问题，我看到了会评论的。

---

总复习

下面是代码体中的一般观察和建议，不一定与主要缺陷相关。然而，他们中的大多数人指出的问题可能会导致未来的缺陷。

<<p> 变量初始化/strong>

关于一般编码实践。永远不要声明未初始化的指针。理想情况下，永远不要声明未初始化的。像这样:

StockNodePtr head, new, current, previous, nextStock;
unsigned listSize;

可以反过来咬你的屁股。应该是这样的:

StockNodePtr head=NULL, new=NULL, currentNULL, previousNULL, nextStockNULL;
unsigned listSize = 0;

如果您担心初始化一个值只是为了在声明之后立即重写它，不必担心。编译器会为你优化(不包括volatile)。

链表构造

构造尾追加链表的一个常见问题是知道最后一个"下一个"指针在哪里。人们经常混淆两个或三个指针，特殊情况下头指针在初始结构等。我在这里告诉你，这些都不需要。

考虑dbl指针(保存另一个指针变量地址的指针)在构造过程中如何非常有效地辅助尾追加列表:

StockNodePtr head = NULL;
StockNodePtr *next = &head; // points to the next pointer to assign.
while(not finished)
{
    StockNodePtr newNode = malloc(sizeof(*newNode));
    // ...
    // set all your fields.
    //
    // whatever pointer `next` points to gets the new node. on an
    //  initial list it will be the `head` pointer. on a subsequent
    //  node it will be the `nextStock` pointer of the last-node-added.
    *next = newNode;
    // now just set the new next-pointer-to-populate to be the `nextStock`
    //  pointer of the node we just added.
    next = &newNode->nextStock;
}
// terminate the last node
*next = NULL;

前面代码中的next指针总是保存下一个要填充的指针的地址。最初它是用head指针变量的地址填充的。当循环结束时，无论next指向的指针是什么，都需要设置为NULL以终止列表。注意:设置字段时不需要设置newNode->nextStock = NULL;。循环的下一次迭代将为您设置它(到下一个节点)，或者如果它是最后添加的节点，循环后的*next = NULL;将把它设置为NULL。

---

我想深入研究strtok()和所有那些指针的用法，但是我晚餐要迟到了，我的另一半打电话来了。我希望这至少对你有所帮助。祝你好运，并有一个爆炸性的一天。