[plpython] Add missing volatile qualifier.
Hi hackers,
I'm playing a toy static analysis checker with PostgreSQL and found a
variable is missing volatile qualifier.
Best Regards,
Xing
Attachments:
v1-0001-Add-missing-volatile-qualifier.patchtext/x-patch; charset=US-ASCII; name=v1-0001-Add-missing-volatile-qualifier.patchDownload+1-2
Xing Guo <higuoxing@gmail.com> writes:
I'm playing a toy static analysis checker with PostgreSQL and found a
variable is missing volatile qualifier.
Good catch! It looks like the consequences of a failure would be
pretty minimal --- AFAICS, no worse than a possible failure to remove
a refcount on Py_None --- but that's still a bug.
I don't care for your proposed fix though. I think the real problem
here is schizophrenia about when to set up pltargs, and we could
fix it more nicely as attached. (Perhaps the Asserts are overkill
though.)
regards, tom lane
Attachments:
fix-unsafe-initialization-of-pltargs.patchtext/x-diff; charset=us-ascii; name=fix-unsafe-initialization-of-pltargs.patchDownload+8-4
On Mon, Apr 01, 2024 at 11:57:07AM -0400, Tom Lane wrote:
Xing Guo <higuoxing@gmail.com> writes:
I'm playing a toy static analysis checker with PostgreSQL and found a
variable is missing volatile qualifier.Good catch! It looks like the consequences of a failure would be
pretty minimal --- AFAICS, no worse than a possible failure to remove
a refcount on Py_None --- but that's still a bug.
Huh. I seem to have dropped that "volatile" shortly before committing for
some reason [0]/messages/by-id/20230504234235.GA2419591@nathanxps13.
I don't care for your proposed fix though. I think the real problem
here is schizophrenia about when to set up pltargs, and we could
fix it more nicely as attached. (Perhaps the Asserts are overkill
though.)
Your fix seems fine to me.
[0]: /messages/by-id/20230504234235.GA2419591@nathanxps13
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Nathan Bossart <nathandbossart@gmail.com> writes:
On Mon, Apr 01, 2024 at 11:57:07AM -0400, Tom Lane wrote:
Good catch! It looks like the consequences of a failure would be
pretty minimal --- AFAICS, no worse than a possible failure to remove
a refcount on Py_None --- but that's still a bug.
Huh. I seem to have dropped that "volatile" shortly before committing for
some reason [0].
Oh, I'd forgotten that discussion. Given that we were both confused
about the need for it, all the more reason to try to avoid using a
within-PG_TRY assignment.
Your fix seems fine to me.
Thanks for looking, I'll push it shortly.
regards, tom lane