Program crashes when using pointer to pointer

[Milun]

Ok, I've been stumped for hours.

Basically, I've got 4 dots on the screen (dot is a class). I want to make it possible to click and drag the dots, but if there is one or more dots overlapping, clicking will move them both, and they cannot be separated. Naturally, I make a pointer to the dot that's currently selected to prevent this (the pointer is declared in main()). Now, the dot class has a private attribute, which is:

Dot** ptr;

And in its constructor, the main() pointer is passed as the address like so:

main()

Dot* ptr = NULL;
Dot(ptr, 40, 40);

Dot.h

inline Dot(Dot* _ptr, int _x = 0, int _y = 0) {ptr = &_ptr; x = _x; y = _y;};

Now, what I have Dot do every step is:

If the mouse pointer is within range, and the pointer doesn't point to any other dots, set the pointer to point to this and then move to the mouse pointers position:

    if ( sel() && ptr != NULL && (*ptr) == NULL ) (*ptr) = this;
    
    if ( ptr != NULL && (*ptr) == this) (*ptr)->setXY(mouse_x, mouse_y);

Now, all this works fine, until I do this:

If the mouse button is not pressed, make the pointer = NULL, a.k.a, no dot is selected.

    if (!mouse_b & 1)
    {
        if (ptr != NULL && (*ptr) != NULL) free(*ptr);
    }

This code crashes the program every time, and if I comment it out it runs, and logically the first dot clicked on should be movable, but it isn't. This is my first time using double pointers, could someone please tell me if I'm just making a stupid mistake?

Thanks very much.

EDIT: IGNORE the "if (!mouse_b & 1)". I am using Allegro, which uses such odd code.

[shacktar]

could someone please tell me if I'm just making a stupid mistake?

If the Dot* was not allocated with malloc or calloc then you shouldn't call free on it. Setting *ptr = NULL would be sufficient in your case, I would think.

And I'm not sure if this is what you want:

if (!mouse_b & 1)

That would be true if the negation of mouse_b bitwise-and with 1 is not zero.

[Vlykarye]

If he is using classes, then he is programming in C++.
If he is programming in C++, then he shouldn't be using malloc or free either way.

The single & may or may not be seen as a && depending on your compiler.
Otherwise, the & only means that both expressions will be checked, so it has absolutely no effect on your if statement.

If you want a good answer, then I think it would be better to post more information on your Dot class.

[Milun]

Oh dear. Ok, the reason I try to free(*ptr) is because earlier, I tried making it = NULL and the program still crashed. You don't need to worry about the if !(mouse_b & 1) thing, it's for Allegro and is not the problem.

Here are all my Dot class methods, but I'm not sure how they'd help:

#include "Dot.h"

void Dot::draw(BITMAP* bmp_scrBuffer)
{
     line(bmp_scrBuffer, x-3, y-3, x+3, y+3, makecol(0,0,0));
     line(bmp_scrBuffer, x+3, y-3, x-3, y+3, makecol(0,0,0));
     move();
}

bool Dot::sel()
{
    static bool press = false;
     
    if ((mouse_b & 1) &&
        mouse_x > x-20 &&
        mouse_x < x+20 &&
        mouse_y > y-20 &&
        mouse_y < y+20 &&
        !press)
    {
        press = true;
        return true;
    }
    
    if (!mouse_b & 1)
    {
        press = false;
        if (ptr != NULL && (*ptr) != NULL) (*ptr) = NULL;
    }
    
    return false;
}

void Dot::move()
{
    if ( sel() && ptr != NULL && (*ptr) == NULL ) (*ptr) = this;
    
    if ( ptr != NULL && (*ptr) == this) (*ptr)->setXY(mouse_x, mouse_y);
    //if (!mouse_b & 1)   (*ptr) = NULL;
}

[Vlykarye]

In C++, there should be no NULL. There should only be new and delete. (Unless you are blending C++ with C, then you might have a reason to use NULL and free and malloc and etc)

You should create a function or class to control mouse functionality. If you have every Dot class controlling functionality, then that's a lot of wasted checking.

Now, knowing what you are doing in the class helps for many reasons. Not only can someone answer your question, they can also provide feedback to optimize your code, or tell you a better way, like I did above.

As for your problem, I cannot really answer you without knowing if what I am saying is correct. I have only been programming in C++ for maybe half a year, and I have little knowledge of inline functions. I don't know if they are treated differently than normal functions other than the fact that the compiler can optimize some of your code.

What I do know is that

Dot* ptr = NULL;
Dot(ptr, 40, 40);

should cause an error because there have been no Dot objects declared, and there are no other Dot() functions in what you provided. But, if that line is being replaced by the inline Dot function, then I sorta see what is happening.

Also, wouldn't
(*ptr) = NULL;
release the memory holding your Dot objects?

[rollie]

First a small correction - NULL is most certainly useful in C++ programming. Setting a pointer to NULL after 'delete'ing it guarantees the object won't be deleted a second time. That causes bad things to happen. Toward this end, a new version of NULL for pointers was explicitly added to the newly released c++ x11 standard.

Onto the program in question. As a style note, please do not name your pointers 'ptr'. It's not very descriptive :P I've seen a lot of examples where people will prefix their variable names with 'p' or 'ptr' to make the code easier to read, but still give your variables useful names.

For the dot movement, I would definitely get rid of the "ptr" variable from the dot class - that's not the way to go about solving this. I would instead do something like:

// Includes here

class Dot
{
public:
	Dot(){ m_nX = m_nY = 0; }
	Dot(int nX, int nY)
	{ 
		m_nX = nX;
		m_nY = nY; 
	}

	void setXY(int nX, int nY)
	{
		m_nX = nX;
		m_nY = nY; 

		// You may do more here, not sure what else was in this function
	}

	// All dots are apparently 40x40 squares.  To improve, make this customizable!  
	// And maybe even allow round dots, if you want to look up the math to determine if a point is in a circle
	bool HitTest(int nMouseX, int nMouseY)
	{
		if (nMouseX > m_nX - 20 &&
			nMouseX < m_nX + 20 &&
			nMouseY > m_nY - 20 &&
			nMouseY < m_nY + 20)
		{
			return true;
		}

		return false;
	}
	
protected:
	int m_nX, m_nY;
};

vector<Dot *> g_Dots;
Dot * g_pSelectedDot = NULL;

void main()
{
	g_Dots.push_back(new Dot(1, 1));
	g_Dots.push_back(new Dot(7, 3));
	g_Dots.push_back(new Dot(2, 5));

	// Put hooks to event handlers here

	// Cleanup
	for (vector<Dot *>::iterator itr = g_Dots.begin(); itr != g_Dots.end(); itr++)
	{
		delete (*itr);
		(*itr) = NULL;
	}
	g_Dots.clear();
}

// Event handlers
void OnMouseMove(int nX, int nY)
{
	if (g_pSelectedDot)
	{
		g_pSelectedDot->setXY(nX, nY);
	}
}

void OnLButtonDown(int nX, int nY)
{
	for (vector<Dot *>::iterator itr = g_Dots.begin(); itr != g_Dots.end(); itr++)
	{
		if ( (*itr)->HitTest(nX, nY) )
		{
			g_pSelectedDot = (*itr);
			break; // Get us out of the for loop
		}
	}
}

void OnLButtonUp(int nX, int nY)
{
	g_pSelectedDot = NULL;
}

[Milun]

YES! Finally. It took some tampering, but after abolishing with the whole double pointer thing, and just having mouse handle everything as you said I managed to get it running.

If anyone's interested as to how, it went like this:

I had another class (that I neglected to mention because it was irrelevant) called Hitbox, which was basically a line between two points. Now, Hitbox stored two Dots, and I just gave it a function like this:

Dot* Hitbox::getPoint()
{
    if      (A.sel()) return &A; //If the mouse cursor is in range of A
    else if (B.sel()) return &B; //If the mouse cursor is in range of B
    else              return NULL; //If neither return NULL
}

Then, I just had the main() cycle through all the Hitboxes, running that function and assigning the return value, assuming it was not NULL. Granted it has a bias in the event of an overlap in Dots towards the Hitboxes made later, but for my goal it is suitable.

Thank you very much for your help!

[Vlykarye]

Alright. Let me restate what I said. You should avoid using NULL, *unless you know what it has been defined as. Don't blame C++ if somewhere in some library or other resource someone else decided to use NULL for something else (even if it's a rare case). Use of 0 or nullptr (if your compiler supports it) could prevent this.

As for everything else rollie said, I agree.

Could you explain your Hitbox class more? Sounds interesting. Will you by chance be posting your finished executable anywhere? I would love to see how it came out.

[Milun]

Really? Well sure I'll post it. Just keep in mind, I wrote this with Allegro (due to not being able to figure out how to draw a line in SDL), so the mouse pointer glitches on the screen... and it has a 50% chance to crash when you press ESC to close it.

http://www.mediafire.com/?fls2s1tnhbq4g55

Here's the .exe and source code. This isn't really "finished" per-say, as the final will check for their intersection, as well as if their radius (a bubble drawn around the perimeter in the line in a pill shape) intersect. You'll probably want to ignore all the Allegro exclusive functions in there though.

[bluecoder]

I doubt about the way the object have been deleted as ..

for (vector<Dot *>::iterator itr = g_Dots.begin(); itr != g_Dots.end(); itr++)
	{
		delete (*itr);
		(*itr) = NULL;
	}

This line seems to be little fishy ..
please enlighten me ..

thanks in advance .
xx

[rollie]

bluecoder, all of the Dots in the g_Dots vector were added with

g_Dots.push_back(new Dot(x, y));

Since each one is dynamically allocated with new, they need to be deleted as well (although truthfully it matters less since the program is exiting, still good policy).

[Peter87]

@Vlykarye, & is never seen as &&. & is bitwise AND.

Dot(ptr, 40, 40); is not an error. It creates a temporary Dot object that will only exist on that line. Not very useful because it is never used not an error.

[Vlykarye]

@Peter87, depending on the compiler, some compilers will switch & with && if it believes it is fit. In the case above, how would I have any clue that he was using allegro? I didn't even think about that. I simply stated what it could have been doing. It's common knowledge what & is. In the end though, the result is much the same as &&, except that & will bitwise and the two sides. The result is "practically" the same as saying "it checks both sides whether or not the first side is false." I'm not trying to get technical about it. Plenty of books and sites cover that, and the OP should have already read it somewhere if he's using it. Now, my post was directed toward's the second poster, who as well, probably didn't realize the OP was using allegro. So. MY BAD for not being technical.

As for the topic, the OP has already resolved the original problem, which you all would have noticed if you read all the posts (and there isn't even that many posts to read). The only thing that needs to posted for this topic now are comments on Milun's project, not the posts for the original question.

[firedraco]

depending on the compiler, some compilers will switch & with && if it believes it is fit. In the case above, how would I have any clue that he was using allegro?

Uh, what!?!? If any compiler does that then it's horrible and shouldn't be used.

[Milun]

Oh dear. Sorry for causing so much anger guys, I guess I really should have mentioned I was using Allegro in the original post. I just didn't think it was that important.

Thanks for all your help though, and please don't be mad.

[Vlykarye]

Lol. I know that would be bad! But some people keep saying DevC++ is a good IDE.

You can't control everything man. It is always better to prepare for the worse than to be sorry later. And just to give you an example of what I mean. I read what I stated in an actual book that is supposed to teach people how to program in C++. So yeah. lol

Anyways. Sorry again Milun. I am done now. Good luck with your project!