I think I've found a bug.

[4.20] 'd_shadow_box_proc' crashes 'popup_dialog'
under certain circumstances (details below code window):

"popup_crash.c"

I compiled this using mingw32 version 3.4.2

gcc -O2 -Wall -c popup_crash.c
gcc -o popup.exe *.o -mwindows -lalleg
gcc -o popupd.exe *.o -mwindwos -lalld

So popup.exe was linked to the release version of the Allegro 4.20 library, while popupd.exe was linked to the debug version.

Now what this should demonstrate is that running popup.exe will show two popup dialogs one after another and then exit normally.

popupd.exe should crash on the second popup, which I believe is directly related to the d_shadow_box_proc, which dimensions are 0 in that second popup dialog.

I get the same behaviour when I compile everything with MSVC 2005,(crash in debug version, no crash in release version) so I believe this is a bug in Allegro.

not necessarily important background info: (I found this on investigating a crash in one of my apps that never happened in the release build and always in the debug build. The reason why my d_shadow_box_proc s are per default dimensioned as 0, was simply that i'm using a custom d_stretch_dialog_proc as the second last object in each of my dialogs to adjust the dimensions of the first object to always be large enough to include all other objects.)

Well windows says "Runtime Error! Application terminated in an unusual way." and I thought that's the same as a crash.
>I< am not trying to create the 0 sized bitmap in this case.
create_bitmap(0,0) is called from popup_dialog in this special above mentioned case.
Why it doesn't terminate abnormally in the release version is beyond my scope.

There is another thing that should get corrected in popup dialog though.
That 'bmp' in there is always created with the dimensions of the first dialog object, which leads to wrong behaviour if there are any self-stretching objects(like in my application) that change their dimensions on dialog initialization(when they receive the message MSG_START).
Therefore popup_dialog might want to send that MSG_START message to the dialog, before creating that 'bmp' to lead to correct behaviour.

Of course I could send that message myself before calling popup_dialog.
(and I will do that to prevent the error in my app for now..)
I think that popup_dialog should be responsible for that though.

for viewing convenience, here's a copy of 'popup_dialog' in src/gui.proc

"...before drawing the dialog..." yes.
"...so all other objects should lie within this one..." that's why I have self-stretching dialog objects that stretch on dialog initialization, because I relied on:

What would be wrong about adding a single line, in front of 'bmp = create_..' that says something like if(dialog!=NULL) dialog_message(dialog,MSG_START,...);?
Would that be unsafe in any way?

Which is what it does. What it does not do is send MSG_START again if it redraws the dialog.
That said, it's fairly easy to implement your own version of pop_dialog() or do_dialog() that deals with this.

The error isn't caught. In my experience, 0-sized bitmaps don't (always) generate a crash; they're still not allowed though.

What would be wrong is that do_dialog() already does that. As I said, you can easily write your own wrapper that does more advanced stuff if youwant to do that, but doing this from within Allegro's function changes the behavior of the function, which might break existing programmes.

I did not think it would do that before every redraw. I'm not having an issue with that. I just assumed that MSG_START is send at least once, before the dialog is displayed for the first time.

I still think that popup_dialog is not functioning properly, let me explain:

popup_dialog saves a portion of the bitmap the dialog is to be drawn onto before running the dialog(via do_dialog) to later restore this portion.

So popup_dialog should be conceptually related to anything that's related to displaying the dialog correctly and to clean up (restoring) the display area underneath it after running it.

If a dialog object does stretch itself, because it assumes that (as mentioned in the manual) it gets a one time chance to initialize itself before the dialog is displayed for the first time then the saving of that screen portion should respect that, because the dimensions of an object are directly display behaviour related and the saving of a bitmap portion to later restore it, is also directly display behaviour related.

I do understand now by your arguments that sending MSG_START twice could break stuff, but that doesn't change my mind that popup_dialog is behaving wrong, because of the aforementioned logic(display behaviour) which I think is a valid point.
popup_dialog should just send MSG_START once then and not make use of do_dialog to prevent sending it twice.
I guess this could be done by just copying the do_dialog code into popup_dialog and then adding the bmp creation and restoring in that copy, after MSG_START was send once to the dialog.

The easiest way out of the issue for me, as suggested by KittyCat on IRC, is to not use popup_dialog and instead use do_dialog and just redrawing the previous dialog below it after it exits. The current implementation of popup_dialog feels obsolete to me then, because it does not respect objects that stretch on initialization of a dialog and thus can't save and restore the used display space properly.:-/
(Stretching objects is not too uncommon I think. Just think of objects that stretch to fit the text inside them(in turn related to font size).)

I agree though that I have to write my own function now or use do_dialog. I would just appreciate it, if my arguments were at least heard and understood right.

That would be hackish. I prefer to write a proper implementation of popup_dialog instead, which is easier because, as you said, all the code is already there. I just need to copy and modify it slightly.:)
I'm not actually asking for advice how to fix my code, I'm more like trying to convince you guys that the current implementation of popup_dialog is logically faulty with respect to the specific display-behaviour(proper saving/restoring of a display area) which it is supposed to provide.

Not that this would help but I've been working on a non-blocking windowing system for allegro that uses modified versions of the dialog procedures and uses an AL_DESKTOP structure to store all the global menu, window and dialog stuff. It's pretty cool because it uses the allegro style objects (slightly modified) so it would be easy to create a window template but it's no where near completion. If anyone has any ideas or info on how coding a windowing gui let me know but I'm out for the day so I'm marking this thread just in case.

You can, there's no problem with the GUI code per se; but it's impossible to foresee all possible things a user might want to do in a function like popup_dialog(). popup_dialog() stumbles on dialogs that do not have the size that they are declared with, or are drawn to a different location. It's just not designed to handle those situations.

Right, so the order is undefined by the documentation.

The latter certainly, the former depends on how you word it.
I would alter the documentation to something like:

I maintain that the current implementation is fine: it does what it's supposed to do.

Good. You're not, you know.

Probably. However, it will duplicate a lot of code and it means possible fixes to do_dialog() would need to be applied to both functions, making it harder to maintain.

You can do it without changing popup_dialog() by using a custom dialog object that handles the creation and maintenance of the backbuffer.