[rfc] Return old cleanup when doing a restore_cleanups()

[rfc] Return old cleanup when doing a restore_cleanups()

[Andrew Cagney]

Hello,

The attached patch tweeks the restore_cleanups() family of functions so 
that the old cleanup is returned.

In theory, that returned cleanup should be NULL (since the cleanup chain 
will have been drained just prior to the restore_cleanups() call).  In 
reality, I'm not too sure.  I'd like to find out what the old chain was 
so I can add an assert to certain callers (e.g. catch_errors()).

I guess the alternative is to just stick the assert in 
restore_cleanups() and see what breaks.

Thoughts?  Preference?

	Andrew

Ref: top.c:catch_errors()

   /* FIXME: cagney/1999-11-05: A correct FUNC implementation will
      clean things up (restoring the cleanup chain) to the state they
      were just prior to the call.  Unfortunately, many FUNC's are not
      that well behaved.  This could be fixed by adding either a
      do_cleanups call (to cover the problem) or an assertion check to
      detect bad FUNCs code. */

   /* Restore the cleanup chain and error/quit messages to their
      original states. */

   restore_cleanups (saved_cleanup_chain);

Re: [rfc] Return old cleanup when doing a restore_cleanups()

[Kevin Buettner]

On Aug 13,  3:28pm, Andrew Cagney wrote:

> The attached patch tweeks the restore_cleanups() family of functions so 
> that the old cleanup is returned.
> 
> In theory, that returned cleanup should be NULL (since the cleanup chain 
> will have been drained just prior to the restore_cleanups() call).  In 
> reality, I'm not too sure.  I'd like to find out what the old chain was 
> so I can add an assert to certain callers (e.g. catch_errors()).
> 
> I guess the alternative is to just stick the assert in 
> restore_cleanups() and see what breaks.
> 
> Thoughts?  Preference?

At the moment, the only caller of restore_cleanups() is
catch_errors(), so the assert could just as easily go in
restore_cleanups() without causing any more breakage than placing the
assert in catch_errors().

If all (future) callers of restore_cleanups are supposed to drain the
cleanup chain first, then I think the assert belongs in
restore_cleanups().  If you think there might be a good reason to
violate this rule, then the assert belongs in the caller.  Personally,
I'd rather see the assert go in restore_cleanups() until someone can
demonstrate a good reason for not draining the cleanup chain.

Kevin

Re: [rfc] Return old cleanup when doing a restore_cleanups()

[Andrew Cagney]

> At the moment, the only caller of restore_cleanups() is
> catch_errors(), so the assert could just as easily go in
> restore_cleanups() without causing any more breakage than placing the
> assert in catch_errors().

Oops, didn't notice that detail.

> If all (future) callers of restore_cleanups are supposed to drain the
> cleanup chain first, then I think the assert belongs in
> restore_cleanups().  If you think there might be a good reason to
> violate this rule, then the assert belongs in the caller.  Personally,
> I'd rather see the assert go in restore_cleanups() until someone can
> demonstrate a good reason for not draining the cleanup chain.

Yes, it is more being convinced that having the assertion there won't 
mysteriously break something ... :-/

I guess we're sufficiently early in the 5.2 release cycle to do it the 
right way.  I'll suck it and see.

Patch withdrawn.

	Andrew