PPP chapter 16 exercise 4 - read access violation.

[grumblesnake]

I'm currently trying to complete exercise 4 in Stroustrup's Programming Principles and Practice, which have you create a window class able to create a button, which when clicked creates a graphical shape of specific proportions.
My idea was to create buttons with callback functions that move a hidden button to a position designated by input from an inbox, I then want the button to produce the shape when clicked.
I wanted it to produce a new shape object of the designated type, which I guess required pointers, a subject that is the topic of chapter 17.
The program runs, but when I clicked a button to move the button based on the input, it throws an exception.

  Exception thrown: read access violation.
this was 0xCCCCCCCC.

I suspect it has something to do with my use of pointers, since it is something I've yet to really examine in any depth, but I wanted to at least have someone else look at it before I simple decide to forgo creating a new shape and just have the program resize and change the position of existing shapes.

I'm linking a github repository containing all the headers and source files I'm using. The class in question (Item_window) is in "My_window".

https://github.com/tattermunge/PPP-C16E04

Any help would be much appreciated.

[ne555]

disclaimer: I don't have any idea on GUIs

running through gdb

(gdb) backtrace
#0  0x00007ffff7ed7bd0 in Fl_Input_::static_value(char const*, int) () from /usr/lib/libfltk.so.1.3
#1  0x00007ffff7ed7da1 in Fl_Input_::value(char const*, int) () from /usr/lib/libfltk.so.1.3
#2  0x000055555555a393 in Graph_lib::Out_box::put (this=0x7fffffffdfe0, s="Point: (1, 2)\nradius: 3")
    at GUI.cpp:46
#3  0x000055555556d1f1 in Graph_lib::Item_window::set_clb (this=0x7fffffffdc00) at My_window.cpp:308
#4  0x000055555556c0d0 in Graph_lib::Item_window::<lambda(Graph_lib::Address, Graph_lib::Address)>::operator()(Graph_lib::Address, Graph_lib::Address) const (__closure=0x0, pw=0x7fffffffdc00) at My_window.cpp:215
#5  0x000055555556c0f8 in Graph_lib::Item_window::<lambda(Graph_lib::Address, Graph_lib::Address)>::_FUN(Graph_lib::Address, Graph_lib::Address) () at My_window.cpp:215
#6  0x00007ffff7f08872 in Fl_Widget::do_callback(Fl_Widget*, void*) () from /usr/lib/libfltk.so.1.3
#7  0x00007ffff7eb8bf5 in Fl_Button::handle(int) () from /usr/lib/libfltk.so.1.3
#8  0x00007ffff7eb0a8a in ?? () from /usr/lib/libfltk.so.1.3
#9  0x00007ffff7eb296e in Fl::handle_(int, Fl_Window*) () from /usr/lib/libfltk.so.1.3
#10 0x00007ffff7f0f96b in fl_handle(_XEvent const&) () from /usr/lib/libfltk.so.1.3
#11 0x00007ffff7f11206 in ?? () from /usr/lib/libfltk.so.1.3
#12 0x00007ffff7f11522 in fl_wait(double) () from /usr/lib/libfltk.so.1.3
#13 0x00007ffff7eb22d6 in Fl::wait(double) () from /usr/lib/libfltk.so.1.3
#14 0x00007ffff7eb23de in Fl::run() () from /usr/lib/libfltk.so.1.3
#15 0x000055555557001e in Graph_lib::gui_main () at Window.cpp:69
#16 0x0000555555559808 in main () at C16E04_main.cpp:9

the last part that it's your code

message_box.put(os.str()); //frame #3, My_window.cpp:308
reference_to<Fl_Output>(pw).value(s.c_str()); //frame #2, GUI.cpp:46 

so it seems that's a problem with `message_box'

looking at the `Item_window' constructor, you didn't `attach()' `message_box' (whatever that is)


by the way, this smells like memory leak

void Item_window::add_circle()
	{
		Circle *new_cl = new Circle{ cl_p, cl_r };
		pos_button.hide();
		attach(*new_cl);
		redraw();
	}

because in the constructor you simply do attach(cl_button);, you are not dereferencing a pointer or allocating memory dynamically
¿so how's going to delete that memory?

[grumblesnake]

running through gdb

Not sure what that means?

so it seems that's a problem with `message_box'

looking at the `Item_window' constructor, you didn't `attach()' `message_box' (whatever that is)

Well that is pretty dumb mistake on my part, I guess that is technically the answer to my query. The message_box was an out_box widget, that simple writes the coordinates entered on the window.

by the way, this smells like memory leak

1
2
3
4
5
6
7



void Item_window::add_circle()
{
Circle *new_cl = new Circle{ cl_p, cl_r };
pos_button.hide();
attach(*new_cl);
redraw();
}


because in the constructor you simply do attach(cl_button);, you are not dereferencing a pointer or allocating memory dynamically
¿so how's going to delete that memory?

That's what I had originally thought was the cause of the error.
A memory leak was been mentioned once in the book, but never actually explained, as I'm guess that is part of chapter 17 or 18.
From what I've gathered, this has to do with the "new" keyword, which allocates memory for the object to the heap, but since memory allocation hasn't been covered in much detail so far, this is mostly meaningless keywords to me.
I guess I should have a destructer for the classes that I'm going to initialize as new pointers?
The objects are all of the base class Shape which has the class members:

Shape(const Shape&) = delete;
Shape& operator=(const Shape&) = delete;

Would this suffice to prevent a memory leak or are they the protection mechanisms I'm circumventing by attaching the objects as new pointers?
If I am causing a memory leak, how do I know whether it is released on program termination?
As it is, I don't think these questions are necessarily relevant, as for some reason the changes I'm making to the position button doesn't seem to take effect, which includes changing the callback function from doing nothing to calling add_circle(), add_rectangle(), add_equitri() or Hexagon(). Thus the new pointers are not actually being allocated in memory anyway.
But if GUI programming isn't your forte, maybe that isn't something you can help me out with.
In any case, I guess my problem is technically solved, but I would appreciate if you could answer some these questions anyway. I thank you for taking the time to help out a programming newb ;)

[ne555]

>> running through gdb
> Not sure what that means?
gdb is a debugger, a wonderful tool used to fix runtime and logic errors
when your program crashes it will stop the execution, showing the line where it happened and the path it took to get there (the backtrace), then you may inspect variables to see erroneous values.

another good tool is valgrind, it's easier to detect invalid access (invalid pointers, out of bounds errors) and bad memory management (double delete, memory leak)


about your questions on memory leaks
seems that you lack a lot of the theory here, so I don't want to confuse you even more.
say that you have a web browser. Each time you want a new tab you do t = new tab(url); that would reserve (new) enough memory to store all the text and images from the web page.
now, when you are done with the page and close the tab you should release (delete) that memory so other programs may use it. Suppose that you fail to do so, then after some hours/days of use, your web browser may is eating gigas of memory but perhaps you are only showing a blank page.

so, for every `new' there should be a matching (same memory address) `delete'.
now, you may see code where you are expected to write like this window.add_widget(new Button("Yes")); because `window' has the responsibility of releasing that memory.

but attach(*new_cl); was kind of weird so it tingled my vinyl little antennas


ja.. found some code
in GUI.h, class Menu

int attach(Button& b);	// attach button; Menu does not delete &b
int attach(Button* p);	// attach new button; Menu deletes p 

now another issue, dangling references
lets say that you instead do

void Item_window::add_circle()
{
	Circle new_cl{ cl_p, cl_r };
	attach(new_cl);
}

when `add_circle()' ends, new_cl is destroyed because it ends its scope
looking at the code of `attach()'

void In_box::attach(Graph_lib::Window& win)
{
	pw = new Fl_Input(loc.x, loc.y, width, height, label.c_str());
	own = &win; //this reference would become invalid
}

> If I am causing a memory leak, how do I know whether it is released on program termination?
monitor the ram.
when the program ends, the OS will recover the memory, the problem is while the program is still running
a similar issue are orphans process, for example, a web browser launches a media player to reproduce a video your browser ends but don't close the media player


> for some reason the changes I'm making to the position button doesn't seem to take effect,
yeah, noticed that
sorry, but can't help you

[grumblesnake]

gdb is a debugger, a wonderful tool used to fix runtime and logic errors
when your program crashes it will stop the execution, showing the line where it happened and the path it took to get there (the backtrace), then you may inspect variables to see erroneous values.

another good tool is valgrind, it's easier to detect invalid access (invalid pointers, out of bounds errors) and bad memory management (double delete, memory leak)

That sounds quite useful, I'll make a note of that for the future.

now another issue, dangling references
lets say that you instead do

void Item_window::add_circle()
{
	Circle new_cl{ cl_p, cl_r };
	attach(new_cl);
}

when `add_circle()' ends, new_cl is destroyed because it ends its scope

This was the reason that I felt that I needed the "new" keyword, but I supposed it wouldn't be enough on its own, as long as the attach function takes a reference as an argument. This was what caused me to search around, and rather carelessly copy a method without truly grasping its function.

ja.. found some code
in GUI.h, class Menu

int attach(Button& b);	// attach button; Menu does not delete &b
int attach(Button* p);	// attach new button; Menu deletes p  

So I assume that means the Menu widget has it's own version of attach that allows it to attach "new" pointer Buttons without creating a memory leak.
I guess I already knew that. I did consider using a Menu, but since it automatically moves the Buttons, I felt it wasn't especially suited for my purpose.
Still, as you correctly surmised, the theory of pointers, memory allocation and memory leaks is still rather vague to me, though, thanks to you, I suppose I'm now a bit more informed on the subject.

I guess there is nothing else to do but go through the code again, and maybe use a less "elegant" method that doesn't involve pointers, until I've been properly introduced to the subject.

I thank you for your time.