user1940516
user1940516

Reputation: 41

Why am I getting a segmentation fault when i set a pointer to an object equal to NULL?

Ok so i have a program that is running essentially a market simulation. I have a lot of my program working just fine, i just have one issue. At a point in my program I have a struct of checkers like so:

 14 struct Checker
 15 {
 16  int m_money_in_register;
 17  int m_start_work;
 18  int m_time_checkout;
 19  Cust *m_cust;
 20 };

I create a pointer to an array of checker objects as follows:

118 Checker *checkers = new Checker[num_checkers]; // Initializing all checkers
119  for(int i = 0; i < num_checkers; i++)
120  {
121   checkers[i].m_money_in_register = 500;
122   checkers[i].m_start_work = 0;
123   checkers[i].m_cust = NULL;
124   checkers[i].m_time_checkout = 0;
125  }

This works just fine, and when checkers[i].m_cust = NULL it means that the checker is available and I can assign a Cust object thats in my checkout_queue to the checker.

My program does what i want it to do for the first Cust that checks out, however when that Cust is done checking out, I want to set checkers[i].m_cust = NULL so that it will be available for a customer once more, however, this is giving me a segmentation fault and it is as follows:

195 if(checkers[i].m_time_checkout == clock && checkers[i].m_cust->get_status() == "shopper" && checkers[i].m_cust != NULL)
196      {
197       int amount_paid = checkers[i].m_cust->get_num_items()*5;
198       checkers[i].m_money_in_register += amount_paid;
199       checkers[i].m_cust->print_done_checkout(cout, clock, amount_paid, i);
200       Cust *tmp = checkers[i].m_cust;
201       delete tmp;
202       num_custs--;
203       checkers[i].m_cust = NULL; // SEG FAULT CAUSED BY THIS
204      }
205
206      if(checkers[i].m_time_checkout == clock && checkers[i].m_cust->get_status() == "robber" && checkers[i].m_cust != NULL)
207      {
208       int checker_cash = checkers[i].m_money_in_register;
209       checkers[i].m_cust->print_done_stole(cout, clock, checker_cash, i);
210       checkers[i].m_money_in_register = 0;
211       Cust *tmp = checkers[i].m_cust;
212       delete tmp;
213       num_custs--;
214       checkers[i].m_cust = NULL; // SEG FAULT CAUSED BY THIS
215      }

How can I correctly set the object to NULL? an example of the output im getting is as follows:

1: Joe entered store
Should be done at: 7
2: James entered store
Should be done at: 8
3: Bob entered store
Should be done at: 9
4: Abby entered store
Should be done at: 10
5: Leo entered store
Should be done at: 11
7: Joe done shopping
7: Joe started checkout with checker 0
time checkout: 13
8: James done shopping
8: James started checkout with checker 1
time checkout: 14
9: Bob done shopping
10: Abby done shopping
11: Leo done shopping
13: Joe paid $10 for 2 items to checker 0
Segmentation fault

Like i said, line 13 is what i want to happen, but setting the pointer to a Cust object equal to null is giving me the seg fault. If any more clarification is needed or you want the GDB message I can provide it. Any input is appreciated, thanks!


The entire loop is as follows:

175    for(int i = 0; checker_queue.empty() != true && i < num_checkers; i++)
176    {
177      int checker_number = i;
178      if(checkers[i].m_cust == NULL && checkers[i].m_start_work == 0)
179      {
180       checkers[i].m_cust = checker_queue.dequeue();
181       checkers[i].m_cust->print_start_checkout(cout, clock, checker_number);
182
183      if(checkers[i].m_cust->get_status() == "shopper")
184       {
185        checkers[i].m_time_checkout = clock + (checkers[i].m_cust->get_num_items()*3);
186       }
187
188       else if(checkers[i].m_cust->get_status() == "robber")
189       {
190        checkers[i].m_time_checkout = clock + 7;
191       }
192       cout << "time checkout: " << checkers[i].m_time_checkout << endl; //check for checkout time
193      }
194
195      if(checkers[i].m_time_checkout == clock && checkers[i].m_cust->get_status() ==    "shopper" && checkers[i].m_cust != NULL)
196      {
197       int amount_paid = checkers[i].m_cust->get_num_items()*5;
198       checkers[i].m_money_in_register += amount_paid;
199       checkers[i].m_cust->print_done_checkout(cout, clock, amount_paid, i);
200       Cust *tmp = checkers[i].m_cust;
201       delete tmp;
202       num_custs--;
203       checkers[i].m_cust = NULL;
204      }
205
206      if(checkers[i].m_time_checkout == clock && checkers[i].m_cust->get_status() == "robber" && checkers[i].m_cust != NULL)
207      {
208       int checker_cash = checkers[i].m_money_in_register;
209       checkers[i].m_cust->print_done_stole(cout, clock, checker_cash, i);
210       checkers[i].m_money_in_register = 0;
211       Cust *tmp = checkers[i].m_cust;
212       delete tmp;
213       num_custs--;
214       checkers[i].m_cust = NULL;
215      }
216     }

If I comment out setting the checkers[i].m_cust = NULL, i get more ouput like as follows:

1: Joe entered store
Should be done at: 7
2: James entered store
Should be done at: 8
3: Bob entered store
Should be done at: 9
4: Abby entered store
Should be done at: 10
5: Leo entered store
Should be done at: 11
7: Joe done shopping
7: Joe started checkout with checker 0
time checkout: 13
8: James done shopping
8: James started checkout with checker 1
time checkout: 14
9: Bob done shopping
10: Abby done shopping
11: Leo done shopping
13: Joe paid $10 for 2 items to checker 0
14: James paid $10 for 2 items to checker 1
Segmentation fault

Upvotes: 0

Views: 757

Answers (1)

kfsone
kfsone

Reputation: 24249

I suspect the problem is that you are testing optimized code and so the representation of where the crash happening is incorrect. The problem is likely this code in your if statements:

checkers[i].m_cust->get_status() == "robber" && checkers[i].m_cust != NULL

You have this the wrong way around; you need to check that m_cust is NOT null before you try to call get_status() on it

checkers[i].m_cust != NULL && checkers[i].m_cust->get_status() == "robber"

i.e.

if(checkers[i].m_time_checkout == clock && checkers[i].m_cust != NULL && checkers[i].m_cust->get_status() == "robber")

When given two logical AND conditions:

if ( a && b )

b only needs to be checked if a is true. C and C++ stop evaluating conditions as soon as they reach a condition which renders the rest of the tests unnecessary (called "short circuit evaluation").

if ( a || (b && c) )

If a is true, b and c won't be evaluated. c will only be evaluated if a is false and b is true.

-- Aside --

Cust *tmp = checkers[i].m_cust;
delete tmp;

I'm guessing this is part of your "what's going on here" investigation, but you should be fine doing

delete checkers[i].m_cust;

So long as the object checkers[i].m_cust was allocated individually with "new". Make sure that you aren't doing

Customers* customer = new Customer[someNumber];
...
checkers[i].m_cust = customer[x];
...
delete checkers[i].m_cust;  // << BAD

If your customers are being array-allocated like this, don't delete them individually, use

delete [] customers;

when you're done with them.

Upvotes: 4

Related Questions