Horton

Thread

Commits

  1. Fix volatile vs. pointer confusion

  1. Fix volatile vs. pointer confusion

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-03-11T07:23:39Z

    Variables used after a longjmp() need to be declared volatile.  In
case of a pointer, it's the pointer itself that needs to be declared
volatile, not the pointed-to value.  So we need

    PyObject *volatile items;

instead of

    volatile PyObject *items;  /* wrong */

Attached patch fixes a couple of cases of that.  Most instances were
already correct.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  2. Re: Fix volatile vs. pointer confusion

    Michael Paquier <michael@paquier.xyz> — 2019-03-11T08:31:33Z

    On Mon, Mar 11, 2019 at 08:23:39AM +0100, Peter Eisentraut wrote:
> Attached patch fixes a couple of cases of that.  Most instances were
> already correct.

It seems to me that you should look at that:
https://www.postgresql.org/message-id/20190308055911.GG4099@paquier.xyz
They treat about the same subject, and a patch has been sent for this
CF.
--
Michael

  3. Re: Fix volatile vs. pointer confusion

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-03-11T11:57:39Z

    On 2019-Mar-11, Peter Eisentraut wrote:

> Variables used after a longjmp() need to be declared volatile.  In
> case of a pointer, it's the pointer itself that needs to be declared
> volatile, not the pointed-to value.  So we need
> 
>     PyObject *volatile items;
> 
> instead of
> 
>     volatile PyObject *items;  /* wrong */

Looking at recently committed 2e616dee9e60, we have introduced this:

+       volatile xmlBufferPtr buf = NULL;
+       volatile xmlNodePtr cur_copy = NULL;

where the pointer-ness nature of the object is inside the typedef.  I
*suppose* that this is correct as written.  There are a few occurrences
of this pattern in eg. contrib/xml2.


-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  4. Re: Fix volatile vs. pointer confusion

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-03-13T09:13:28Z

    On 2019-03-11 09:31, Michael Paquier wrote:
> On Mon, Mar 11, 2019 at 08:23:39AM +0100, Peter Eisentraut wrote:
>> Attached patch fixes a couple of cases of that.  Most instances were
>> already correct.
> 
> It seems to me that you should look at that:
> https://www.postgresql.org/message-id/20190308055911.GG4099@paquier.xyz
> They treat about the same subject, and a patch has been sent for this
> CF.

I'm aware of that patch and have been looking at it.  But it's not
directly related to this issue.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  5. Re: Fix volatile vs. pointer confusion

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-03-13T09:14:32Z

    On 2019-03-11 12:57, Alvaro Herrera wrote:
> Looking at recently committed 2e616dee9e60, we have introduced this:
> 
> +       volatile xmlBufferPtr buf = NULL;
> +       volatile xmlNodePtr cur_copy = NULL;
> 
> where the pointer-ness nature of the object is inside the typedef.  I
> *suppose* that this is correct as written.  There are a few occurrences
> of this pattern in eg. contrib/xml2.

I think this is correct, but I don't want to wreck my sanity trying to
understand the syntax-level details of why.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  6. Re: Fix volatile vs. pointer confusion

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-03-14T07:48:09Z

    On 2019-03-11 08:23, Peter Eisentraut wrote:
> Variables used after a longjmp() need to be declared volatile.  In
> case of a pointer, it's the pointer itself that needs to be declared
> volatile, not the pointed-to value.  So we need
> 
>     PyObject *volatile items;
> 
> instead of
> 
>     volatile PyObject *items;  /* wrong */
> 
> Attached patch fixes a couple of cases of that.  Most instances were
> already correct.

Committed.

I'll wait for the build farm to see if there are any new compiler
warnings because of this, then backpatch.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services