SEGFAULT on NULL Pointer...

[trentkg]

I am using C++ to break down soccer games. I have games broken down into the struct "Events," as such:

struct Event {
	unsigned int scoreA; //score of teamA at any point in time
	unsigned int scoreB; //score of teamB 
	unsigned int STime; //start/end time of the event
	unsigned int ETime;
	struct Game *thegame; //pointer to a game the Event is in
	struct Event *nextevent; //pointer to next event in line, or null
	
	}; 

I have a function addEvent that is supposed to add an event to a game, with an auxiliary function LastEvent that returns a pointer to the last event in the game.

typedef struct Event *Eventp;
typedef struct Game *Gamep;

void addEvent( Eventp nwevnt, Gamep g) {
     
		if((g->ev) == NULL) {
		g->ev = nwevnt; 
		++(g->EventNum); 
		}

		else {
        Eventp swtch = LastEvent( g->ev);
        swtch->nextevent = nwevnt;
        ++(g->EventNum);
		}
        
        
     }


Eventp LastEvent ( Eventp EV) { //ev can NEVER be a null pointer, need something to account for this
		
       if((EV->nextevent) == NULL)
        return EV;
		else return LastEvent (EV->nextevent);
		
		
		
       }

In main, I start with a Game that has no events (instead just a pointer to NULL) then try to use addEvent to add in the first game.

int main()
{
   ...
	Eventp first = NULL;
	Gamep PGame = MGame ( "Manu", 1, "Chelsea", 2, 2, 0, first, 0, "This is a practice Game"); 
   ...
	Eventp PEvent = MEvent( 1, 0, 20, 30, PGame, NULL); //creates an Event
	addEvent(PEvent, PGame); //adds Event to the Game, segfaults!!

  ...	
	
	
return 0;
	
    
    
 
    
}

Where obviously the "..." indicates code that is unnecessary so I've blanked it out.

When I run GDB on this code, it says its segfaulting at....

Program received signal SIGSEGV, Segmentation fault.
0x0000000000400c9c in LastEvent (EV=0x100000000) at FIFA1.cpp:45
45 if((EV->nextevent) == 0)

I understand why this is a segfault--the argument EV is a NULL pointer, so obviously EV->nextevent doesn't exist. But, if that is indeed the case, why the heck aren't any of my conditional statements working?? Why doesn't the first line of code in addEvent ( if( g->ev = NULL) ... ) work???

Well, I know why it doesn't work--when I print EV in GDB, it gives the address as 0x10000 blah blah. In other words, the address isn't NULL. WHAT? but I TOLD TOLD TOLD it to be NULL in main when I had the command

Eventp first = NULL

What the heck is going on here?? What am I missing?

[EricDu]

You might want to check if MGame () passes "first" parameter to PGame.ev correctly.

[trentkg]

Is there a way I can do that in the Debugger? All the function MGame () is is....

Gamep /*pointer to the struct "Game"*/ MGame ( string A, unsigned int rA, string B, unsigned int rB, unsigned int fA, unsigned int fB, Eventp e, unsigned int Enum, string n) {
	Gamep a;
	
	a->TeamA = A;
	a->rankA = rA;
	a->rankB = rB;
	a->TeamB = B;
	a->finalA = fA;
	a->finalB = fB;
	a->ev = e;            //this would be the variable of interest no? Seems simple enough....
	a->EventNum = Enum;
	a->notes = n;
	
	return a;
	}

[helios]

LastEvent() might be problematic. You're traversing what looks like a linked list using recursion. If the list is long enough and the compiler doesn't do tail call optimization (which would be odd because I thought GCC did do TCO), you'll end up overflowing the stack.
Try using a loop instead of recursion.

Also, is that the entire code for MGame()? If so, a is pointing to nowhere, so basically everything inside MGame() is incorrect.

The entire thing looks like very unidiomatic C++. It looks more like code directly translated from C.

[trentkg]

helios --

You were right about LastEvent() being a big redundant. Although I don't believe tail recursion is causing my segfault (especially because it should technically only be recursing twice in this instance, at most 7 in a really long game), the addEvent would be much more efficient if it was instead:

void addEvent( Eventp nwevnt, Gamep g) { // adds an event to a game 
     
		if((g->ev) == NULL) {
		g->ev = nwevnt;
		++(g->EventNum); 
		}

		else {
       Eventp temp;
	   temp = g->ev;
	   while(temp != NULL)
	   temp = temp->nextevent; // find the last event 
       temp = nwevnt; 
        ++(g->EventNum);
		}
        
        
     }

where I've now replaced the tail recursive LastEvent() with a simple while() loop. Good tip.

As for MGame(), you're also right in that I completely forgot to allocate memory. It is now...

Gamep MGame ( string Aname, unsigned int rA, string Bname, unsigned int rB, unsigned int fA, unsigned int fB, Eventp e, unsigned int Enum, string nts) {
	Gamep temp;
	new(temp) Game;
	
	
	temp->TeamA = Aname;
	temp->rankA = rA;
	temp->rankB = rB;
	temp->TeamB = Bname;
	temp->finalA = fA;
	temp->finalB = fB;
	temp->ev = e;
	temp->EventNum = Enum;
	temp->notes = nts;
	
	return temp;
	}

Here I have a question. What is the difference between

new (temp) Event;
and

temp = (Eventp) operator new (sizeof (Event);

As my understanding goes, the only difference is that the second call does not call the constructor (if it exists) for the struct, while the first does. Is this it? As long as I never have my constructor doing anything funny (like printing something to the command prompt, as is in most examples of malloc vs. new or the operator call to new and regular new) why should this matter?

Second, can I have constructors in structs?



Likewise, even with the changed code I'm getting a segfault, though in another place. Now when we look in main,

int main()
{
   ...
	
	Gamep PGame = MGame ( "Manu", 1, "Chelsea", 2, 2, 0, first, 0, "This is a practice Game"); 
   ...
	

cout << "\n This Game has these teams: \n" << PGame->TeamA << "   " << PGame->TeamB;
	
return 0;
	
    
    
 
    
}

Now I'm segfaulting at line 9 . Look what happens when I break after line 5 and try to print off the variables of MGame in GDB....

((gdb) print PGame->rankA
$8 = 4293615361 

(gdb) print PGame->rankB
$9 = 0

(gdb) print PGame->finalA
$10 = 2

(gdb) print PGame->EventNum
$11 = 0

(gdb) print PGame->notes
$12 = {static npos = 18446744073709551615,
  _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>},
    _M_p = 0x604068 "This is a practice Game"}}


(gdb) print PGame->TeamA
$13 = {static npos = 18446744073709551615,
  _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, _M_p = 0x6040d8 "Manu"}}

(gdb) print PGame->TeamB
$14 = {static npos = 18446744073709551615,
  _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>},
    _M_p = 0x7fffffffeab000 <Address 0x7fffffffeab000 out of bounds>}}

Obviously something is again wrong with my memory allocation. The only variables that are correct are finalA and finalB. Any clues?


I appreciate all the help!

-Trent

[helios]

Gamep temp;
new(temp) Game;

new (temp) Event;
and

temp = (Eventp) operator new (sizeof (Event);

As my understanding goes, the only difference is that the second call does not call the constructor (if it exists) for the struct, while the first does.

Completely wrong. I suggest you review some material on memory allocation.

Gamep temp=new Game;

Second, can I have constructors in structs?

A struct in C++ is exactly the same as a class, only by default members are public, instead of private. These two are nearly equivalent (the only difference is that a struct/class has to be forward declared as a struct/class):

struct A{
    //...
};

class B{
public:
    //...
};