2009-06-10 75 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。