Nathan Bossart <nathandboss...@gmail.com> writes: > Here's a new version of the patch. Besides adding comments and a commit > message, I made sure to decrement the reference count for pltargs in the > PG_CATCH block (which means that pltargs likely needs to be volatile).
Hmm, actually I think these changes should allow you to *remove* some volatile markers. IIUC, you need volatile for variables that are declared outside PG_TRY but modified within it. That is the case for these pointers as the code stands, but your patch is changing them to the non-risky case where they are assigned once before entering PG_TRY. (My mental model of this is that without "volatile", the compiler may keep the variable in a register, creating the hazard that longjmp will revert the variable's value to what it was at setjmp time thanks to the register save/restore that those functions do. But if it hasn't changed value since entering PG_TRY, then that doesn't matter.) > I'm > not too wild about moving the chunk of code for pltargs like this, but I > haven't thought of a better option. We could error instead of returning > NULL, but IIUC that would go against d0aa965's stated purpose. Agreed, throwing an error in these situations doesn't improve matters. regards, tom lane