1

我创建了一个单链表。一切正常。

我只想知道我是否在我的代码中做了任何潜在危险的事情。我关心的代码片段是我的推送、弹出和清理。代码部分仅用于用户交互,因此并不重要(无论如何我都发布了,以便更清楚地了解我在做什么)。只是链表应用程序。

非常感谢您的任何建议,因为这是我的第一次尝试。

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

typedef struct product_data product_data_t;
struct product_data
{
    int product_code;
    char product_name[128];
    int product_cost;
    product_data_t *next;
};

static product_data_t *head = NULL;
static product_data_t *tail = NULL;
static product_data_t *new_product = NULL;

// Push a product on to the list.
void push(int code, char name[], int cost); 
// Pop (delete) a product from the list.
void pop(int code);
// Display all product in the list.
void display_list();
// Delete all memory allocated on the list
void clean_up();
// Display menu
void menu();

int main(void)
{
    menu();

    getchar();

    return 0;
}

void push(int code, char name[], int cost)
{
    // Allocate memory for the new product
    new_product = calloc(1, sizeof(product_data_t));
    if(!new_product)
    {
        fprintf(stderr, "Cannot allocated memory");
        exit(1);
    }

    /* Populate new products elements fields */
    new_product->product_code = code;
    strncpy(new_product->product_name, name, sizeof(new_product->product_name));
    new_product->product_cost = cost;
    new_product->next = NULL;

    // Set the head and tail of the linked list
    if(head == NULL)
    {
        // First and only product
        head = new_product;
    }
    else
    {
        tail->next = new_product;
    }

    tail = new_product;
}

// Find the product by code and delete
void pop(int code)
{
    product_data_t *product = head;
    product_data_t *temp = NULL;
    product_data_t *previous = head;
    int found = 0; // 0 - Not Found, 1 - Found

    if(!head)
    {
        puts("The list is empty");
        return;
    }

    while(product)
    {
        if(product->product_code == code)
        {
            found = 1; // Found
            // Check if this is in the first node - deleting from head
            if(head->product_code == code)
            {
                temp = head;
                head = head->next;
                free(temp);

                // Finished Deleting product
                return;
            }

            // Check if this is the end node - deleting from the tail
            if(tail->product_code == code)
            {
                temp = tail;
                tail = previous;
                free(temp);

                // Finished deleting product
                return;
            }

            // delete from list if not a head or tail
            temp = product;
            previous->next = product->next;
            free(temp);

            // Finished deleting product
            return;
        }
        // Get the address of the previous pointer.
        previous = product;
        product = product->next;  
    }

    if(!found)
    {
        printf("code [ %d ] was not found\n", code);
    }

    // Set all to null after finished with them
    product = NULL;
    temp = NULL;
    previous = NULL;
}

// Traverse the linked list
void display_list()
{
    // Start at the beginning
    product_data_t *product = head;

    while(product)
    {
        printf("===================================\n");
        printf("Product code: \t\t%d\n", product->product_code);
        printf("Product name: \t\t%s\n", product->product_name);
        printf("product cost (USD): \t%d\n", product->product_cost);
        printf("===================================\n\n");

        // Point to the next product
        product = product->next;
    }
    // Finished set to null
    product = NULL;
}

// Release all resources
void clean_up()
{    
    product_data_t *temp = NULL;

    while(head)
    {
        temp = head;
        head = head->next;
        free(temp);    
    }
    head = NULL;
    temp = NULL;

    // End program - goodbye
    exit(0);
}

void menu()
{
    int choice = 0, code = 0, cost = 0;
    char name[128] = {0};

    do
    {
        fflush(stdin); // Flush the input buffer

        puts("========= Welecome to linked list ===============");
        puts("[1] Add new product to the list");
        puts("[2] Delete a product from the list");
        puts("[3] Display all products");
        puts("[4] Exit and clean up");
        printf("Enter your choice: ");
        scanf("%d", &choice);

        switch(choice)
        {
        case 1:
            printf("Enter product code: ");
            scanf("%d", &code);
            printf("Enter cost: ");
            scanf("%d", &cost);
            printf("Enter name: ");
            scanf("%s", name);
            push(code, name, cost);
            break;

        case 2:
            printf("Enter product code: ");
            scanf("%d", &code);
            pop(code);
            break;

        case 3:
            display_list();
            break;

        case 4:
            clean_up();
            break;

        default:
            puts("Incorrect choice");
            break;
        }
    }while(choice != 4);
}
4

7 回答 7

9

从流行()

            if(head->product_code == code)
            {
                    temp = head;
                    head = head->next;
                    free(temp);

                    // Finished Deleting product
                    return;
            }

在只有一项的情况下,“头”和“尾”将指向同一个节点。但是,如果你弹出这一项,'head' 将被调整,但 'tail' 仍将指向 free'd 节点。这会留下一个错误的指针,这可能会导致您的计算机爆炸。

附录:类似地,如果你弹出最后一个被推送的节点,'new_product' 将悬空,而 clean_up() 也会使 'tail' 指针悬空。即使提供的代码示例在它们被释放后永远不会取消引用它们,C 代码中的悬空指针也应始终被视为“潜在危险”。

于 2009-06-10T04:25:32.917 回答
5
strncpy(new_product->product_name, name, sizeof(new_product->product_name));

如果字符串长于您拥有的大小,则不会正确终止。

于 2009-06-10T04:24:44.443 回答
4

我看不出为什么new_product应该是全球性的,也没有理由不应该是全球性的。

于 2009-06-10T06:01:22.197 回答
3

看起来你在正确的轨道上,但有问题。我会删除全局变量,取而代之的是你传递给函数的 list_t 结构(包含头和尾)。正如其他人所指出的,您可能还希望通过使用(例如)node_t 类型和 void* 数据指针来使列表通用。

一般 push 和 pop 用于指在开头添加或删除项目,而不是任意位置(如您所做的那样);这只是一个命名问题。

如果您使用的是 product_name char *product_name,则可以消除长度限制以及对 strncpy 的需求。您只需让调用者分配字符串,然后在 clean_up 中释放它。

您可以考虑使用枚举来提高菜单的可读性。对于“检查这是否在第一个节点中 - 从头部删除”(尾部相同),您应该只比较头部和产品,而不是比较代码。

在 "tail = previous" 之后,您应该将 tail->next 设置为 NULL。

于 2009-06-10T07:49:26.160 回答
2

同意goldPseudo 和thaggie/Steven 提出的问题。

push()中,替换strncpy()strlcpy()以确保目标字符串始终为 NUL 终止。

cleanup()中,我建议您删除该exit(0);语句 - 您不需要它。从子例程中退出程序通常不是最好的做法。

您应该从创建第一个单链表中吸取教训,也就是说,单链表在现实世界中通常不是很有用,因为:

  • 他们太难操纵了。看看你的pop()子程序的复杂性。
  • 相对较慢,因为每次要从列表中检索元素时都必须从列表的开头开始。

您现在应该尝试编写您的第一个双向链表。虽然双向链表实现起来更复杂,但它们比单链表更容易操作(尤其是在删除元素时)。

于 2009-06-10T05:57:24.017 回答
1

你有什么理由从 clean_up 函数调用 exit(0) 吗?我认为这是潜在的危险,因为您没有给用户机会正确完成程序。

同样,我建议您在构建数据结构时使用数据封装:

typedef struct
{
    int product_code;
    char product_name[128];
    int product_cost;
    list_node *next;
} list_node;

typedef struct
{
   list_node* head;
   list_node* tail;
   list_node* current;
   int        size;
} list;

此外,在列表开头使用跟踪虚拟节点以使代码更通用也是一个好习惯。

于 2009-06-10T06:08:31.183 回答
1

按照正常的命名约定,push 和 pop 与堆栈相关 - 即 push() 应该将一个项目添加到堆栈的顶部(您添加到列表的尾部,这很好!),并且 pop() 应该返回并且从堆栈顶部删除项目(您在列表中的任何位置搜索命名项目并将其删除。)
除了函数名称,我建议对列表进行更通用(抽象)的实现,其中节点的内容是指向任意数据的指针(在您的特殊情况下,稍后将是 product_data)。这样,您的链表可以重复用于任何内容,并且更易于调试、阅读和维护。
最好不要让东西全局化,而是允许列表的多个实例。正常的 C 方法是将数据保存在结构中,然后将实例作为第一个参数传递给每个函数。

于 2009-06-10T06:32:31.777 回答