There is a possible problem with most uses of e_config_dialog, and a
real problem that I found when I added a close button callback.  I
suspect that the problem originated with the first person to write that
code, and then everybody else just cut and pasted it.

In each case I found, E_Config_Dialog_View was allocated on the stack,
and never zeroed out.  Then it is passed to e_config_dialog_new(),
which creates a dialog, then returns.

The real problem is the the lack of clearing out.  I can't rely on the
new close callback pointer being NULL by default, as current code
results in it holding random garbage.  Makes it tricky to add other new
things to the struct for the same reason.  So I explicitly set the
close cb to NULL during e_config_dialog_new(), which means you cannot
pass it in via the E_Config_Dialog_View like all the other callbacks.
If you want to use the close cb you have to do so during your
create_widgets cb -

cfd->view->close_cfdata = func;

Then e_config_dialog checks for NULL before trying to call the close
cb.  My first tests ended up using some random stack garbage as the
callback to call.  That's how I discovered this problem.

The possible problem is the allocate on the stack then return.  Since
the config dialog hangs around the structure allocated on the stack is
used long after the function that allocated it returns.  I know it has
been a long time since I first started coding in C, but I don't think
they have changed stacks that much.  Surely the structure on the stack
is in danger of getting overwritten at some later function call?  It's
a wonder that config dialogs worked at all.  It might explain the
crashes some people have reported though.

Either way, all the current callers of e_config_dialog() should be
changed to at least zero out the E_Config_Dialog_View structure, and
possible to calloc() it instead.  Of course, if you calloc(), you
should free(), which is a problem I feel no urge to solve on a case by
case basis for all current cases.

The actual fix I leave as a matter for discussion.

Attachment: pgpDGd8q92c4L.pgp
Description: PGP signature

Reply via email to