Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Avoid possible longjmp-induced logic error in PLy_trigger_build_args.

  1. [plpython] Add missing volatile qualifier.

    Xing Guo <higuoxing@gmail.com> — 2024-04-01T13:44:31Z

    Hi hackers,

I'm playing a toy static analysis checker with PostgreSQL and found a
variable is missing volatile qualifier.

Best Regards,
Xing

  2. Re: [plpython] Add missing volatile qualifier.

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-04-01T15:57:07Z

    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

  3. Re: [plpython] Add missing volatile qualifier.

    Nathan Bossart <nathandbossart@gmail.com> — 2024-04-01T18:29:20Z

    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].

> 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] https://postgr.es/m/20230504234235.GA2419591%40nathanxps13

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

  4. Re: [plpython] Add missing volatile qualifier.

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-04-01T18:39:05Z

    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