linear linked list errors

Hi all! Could anyone help me to understand what's going wrong when I add to my linear linked list? It works fine when I only have two differently named items (and I can have a lot of each) but once I add a third differently named item the list seems to go bad. The error I get is "Acess Violation" so I'm guessing some part of the list is getting destroyed or something. I've been banging my head over this for a while. Any help is appreciated.

Here is the function.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
int Coupons::add(coupon_info & Info) 
                              //precondition: coupon_info& gives all the data
                              //of the coupon being added. The function then
                              //traverses the list to see where to add the
                              //coupon, then adds it. 
                              //postcondition: coupon_info& is unchanged.
                              //There will be a new coupon in the list.
                              
{
  char temporary[60];
  node * previous = head;
  node * current = head;
  node * temp = new node;
  
  // Allocating memory for all the char variables of the temp node
  strcpy(temporary, Info.store);
  temp->data.store = new char[strlen(temporary)+1];
  
  strcpy(temporary, Info.rules);
  temp->data.rules = new char[strlen(temporary)+1];
  
  strcpy(temporary, Info.expiration_date);
  temp->data.expiration_date = new char[strlen(temporary)+1];
  //End allocation
    
  //Put all the info into the temp node
  strcpy (temp->data.store, Info.store);
  temp->data.discount = Info.discount;
  strcpy (temp->data.rules, Info.rules);
  strcpy (temp->data.expiration_date, Info.expiration_date);
  //End placing info into temp node

  if (!head) //if the list is empty, add the coupon right at the beginning.
  {
  
    head = temp;
    
    head->next = NULL;
  
    return 1;
  }
  
  current = current->next;
  
  if (!current)
  {
    current = temp;
    head->next = current;
    current->next = NULL;
    
    return 1;
  }
    
  current = head;
    
  while (current->next) 
                    //while current still has a node with data inside it,
                    //traverse and see if the store-to-be-added and the current
                    //store have the same name. If they do, add the stba before
                    //the matching name.
  {
    previous = current;
    current = current->next;
  
    if (strcmp (head->data.store, Info.store) == 0)
    {
      temp->next = head;
      head = temp;
      
      return 0;
    }
    
    
    if (strcmp (current->data.store, Info.store) == 0)
    {
      temp->next = current;
      previous->next = temp;  
      
      return 0;    
    }
      

    
    
    if (!current->next) //if there was no match after going through the list,
                        // add the node at the end.
                         
    {
      current->next = temp;
              
      return 0;
    }
    
    
  }
        
  return 0;
  
}
Last edited on
closed account (1yR4jE8b)
for the love of cheese....code tags please!!!

The amount of tag-less posts lately is insane....
Insertion of the second element is trashing the list. I don't know if this is causing the access violation on insertion
of the third item, but since you aren't inserting the second element right, you might as well not worry about three
elements yet.

Line 48 is overwriting the pointer to the second node in the list with a pointer to the newly created node. Not
to mention you aren't performing a sorted insert of the second element anyway.

There is no need to special case insertion of the second element.

On some other stylistic/semantic notes:

1) Lines 16-23 there is no need to make an interim copy of the strings into temporary in order to perform
a strlen() on them.

2) Your comment "//postcondition: coupon_info& is unchanged." would be implied if you were to pass the
coupon info as a const reference instead of a non-const reference. Passing non-const references,
because they have some limitations that const references do not have, implies the function wants to change
the data.

3) You would be much better off using STL std::strings rather than C-style strings (if your project allows it).

Topic archived. No new replies allowed.