1

StackOverflow 上的第一篇文章（行业测试员，业余时间非常糟糕的程序员）。出于披露目的，这是我正在努力解决的大学作业（据我所知，在这里提问并不被禁止。

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

下面的代码，如果有人想看看（哦，更多披露 - 几乎是一个完整的 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 failed\n");
            fprintf(stderr,"Aborting\n");
            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,%i\n", 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
4

1 回答 1

1

主要缺陷

您的基本问题是分配不足，特别是：

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

这意味着您分配的空间足够大以容纳指针；不是股票节点。

试试这个：

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

注意：我还没有彻底阅读剩下的内容，但这本身就相当大，因为你永远不会为每个节点分配比指针大小更多的内存。如果还有其他问题，我会在看到它们时发表评论。

一般审查

以下是代码主体内的一般观察和建议，不一定与主要缺陷有关。他们中的大多数指出了可能导致未来缺陷的问题。

变量初始化

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

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

可以回来真的咬你的屁股。这些应该是这样的：

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

如果您担心初始化一个值只是为了在声明后立即将其写入，请不要担心。编译器将为您优化它（不耐受易失性）。

链表构建

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

考虑一个 dbl-pointer（一个保存另一个指针变量地址的指针）如何在构造过程中非常有效地帮助尾部附加列表：

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()所有这些指针的用法，但我晚饭迟到了，我的另一半正在打电话。我希望这至少有点帮助。祝你好运，并有一个炸药的一天。

于 2013-04-28T00:24:02.243 回答