Thread: Fix volatile vs. pointer confusion
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
Attachment
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
Attachment
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
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
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
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