I wrote: > Took a quick look. I agree that this seems a lot cleaner than the > alternative proposals. I'd suggest however that the header comment > for do_pg_abort_backup could do with more work, perhaps along the > lines of "The odd-looking signature allows this to be registered > directly as a shmem_exit handler".
> Personally I'd have kept the handler as a separate function that's just > a one-line wrapper around "void do_pg_abort_backup(bool emit_warning)". > We don't usually treat callbacks as functions to be also called in > their own right. But if you don't want to do that, I'll settle for an > acknowledgement of the hack in the comment. Oh, scratch that --- looking closer, I see that the only two use-cases in the patched code are via before_shmem_exit and PG_ENSURE_ERROR_CLEANUP, and both of those require a function with the signature of an on_exit callback. So there's no need for a separate wrapper because this isn't going to be called any other way. I still recommend amending the comment to explain why it has this signature, though. regards, tom lane