2009-06-10 73 views
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); 
} 
+0

“一切工作正常。” - 输入长度为129个字符的名称,然后显示列表;-) – 2009-06-10 05:01:22

+0

如何防止某人输入超过128个字符? – ant2009 2009-06-10 07:37:57

+1

@robUK:try scanf(“%127s”,name) – 2009-06-10 16:52:26

回答

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()将离开“尾巴”指针晃来晃去为好。即使提供的代码样本在释放后也不会解除引用,但C代码中的悬挂指针应始终被视为“潜在危险”。

5
strncpy(new_product->product_name, name, sizeof(new_product->product_name)); 

如果字符串比你将无法正确终止尺寸长。

2

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

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

cleanup(),我建议你删除exit(0);声明 - 你不需要它。从子程序中退出程序通常不是最好的选择。

你应该带走一个教训从创建第一个单向链表,那就是,单链表一般都不会在现实世界中,因为非常有用:

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

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

4

我看不出为什么new_product应该是全球性的,以及为什么不应该这样做。

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; 

而且它的使用跟踪虚拟节点在列表的头,使您的代码更加好的做法通用的。

1

遵循正常的命名转换,push和pop与堆栈相关 - 即push()应该添加一个项目到堆栈的顶端(您添加到列表的尾部,这很好!),并且pop( )应该返回,并从堆栈(在列表中搜索一个名为项目的任何地方,并删除它的顶部删除该项目。)
函数名之外,我会建议一个更通用(抽象)实现的列表,其中的一个节点的内容是一个指向任意数据的指针(在你的特殊情况下,它将会是一个product_data)。这样您的链接列表可以重新用于任何内容,并且更易于调试,读取和维护。
不是全局东西,而是允许列表的多个实例也是一个更好的主意。正常的C方法是将数据保存在一个结构中,然后将一个实例作为第一个参数传递给每个函数。

3

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

一般push和pop被用来指添加或开头,而不是任意位置移除项目(如你);这只是一个命名问题。

如果你有PRODUCT_NAME的char * PRODUCT_NAME代替,这将允许你删除长度的限制以及需要strncpy()函数。你只需要调用者分配字符串,然后将其释放到clean_up中。

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

后 “尾巴=前”,你应该设置tail->旁边NULL。