On Thursday 01 May 2008 23:57:58 Daniel Jacobowitz wrote:
> On Tue, Apr 29, 2008 at 10:10:30PM +0400, Vladimir Prus wrote:
> > Here are 3 independent bits.
> > 
> > 1. Introduce the make_cleanup_restore_integer function. You're right
> > that it can lead to bad results if one discards this cleanup, but then
> > one should be careful with discarding cleanups anyway.
> 
> This patch looks fine except that ...
> 
> > 2. Modify the normal_stop observer not to fire in some cases. One
> > case is when doing function call -- we don't announce the stop in CLI
> > and for similar reason we don't have observer to be called. Also,
> > for the benefit of next patch, we want the call to observer to
> > be delayed until we print function return value, if we're doing finish.
> 
> ... unless I'm mistaken you have exactly the memory leak Joel warned
> about, since finish_command discards continuations.
> 
> Am I correct that the cleanup for finish_command is never supposed to
> survive the function returning?  It's run on error and discarded on
> normal return.  So you could put the closure in a local variable,
> maybe.
> 
>   struct foo_closure my_closure = { &my_global, my_global };
>   make_cleanup (restore_integer, &my_closure); 

This is somewhat limiting. Instead, I've implemented a mechanism that
allows a cleanup to well, "cleanup" its argument. I attach a patch
for that.

I also attach a patch stop the 'finish_command_continuation' from
accessing a cleanup is has no business with.

Are those two new patches, together with the previously posted one
(changing stop_observer not to always fire) OK?

- Volodya