pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedFix volatile vs. pointer confusion
6+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2019-03-14 1226d932b4 a1e9508b96 c77f21899a 626c8ed2ef 361cabb11d Fix volatile vs. pointer confusion
* Fix volatile vs. pointer confusion
@ 2019-03-11 07:23 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
0 siblings, 3 replies; 6+ messages in thread
From: Peter Eisentraut @ 2019-03-11 07:23 UTC (permalink / raw)
To: pgsql-hackers
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
Attachments:
0001-Fix-volatile-vs.-pointer-confusion.patch (text/plain, 2.7K) download
From 1bb6acf63cf796d0659283c7ea5220385f7de181 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 11 Mar 2019 08:18:55 +0100
Subject: [PATCH] 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 */
---
contrib/hstore_plpython/hstore_plpython.c | 9 ++++-----
contrib/jsonb_plpython/jsonb_plpython.c | 9 +++------
2 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/contrib/hstore_plpython/hstore_plpython.c b/contrib/hstore_plpython/hstore_plpython.c
index 2f24090ff3..93c39d294d 100644
--- a/contrib/hstore_plpython/hstore_plpython.c
+++ b/contrib/hstore_plpython/hstore_plpython.c
@@ -128,7 +128,7 @@ Datum
plpython_to_hstore(PG_FUNCTION_ARGS)
{
PyObject *dict;
- volatile PyObject *items_v = NULL;
+ PyObject *volatile items = NULL;
int32 pcount;
HStore *out;
@@ -139,14 +139,13 @@ plpython_to_hstore(PG_FUNCTION_ARGS)
errmsg("not a Python mapping")));
pcount = PyMapping_Size(dict);
- items_v = PyMapping_Items(dict);
+ items = PyMapping_Items(dict);
PG_TRY();
{
int32 buflen;
int32 i;
Pairs *pairs;
- PyObject *items = (PyObject *) items_v;
pairs = palloc(pcount * sizeof(*pairs));
@@ -177,14 +176,14 @@ plpython_to_hstore(PG_FUNCTION_ARGS)
pairs[i].isnull = false;
}
}
- Py_DECREF(items_v);
+ Py_DECREF(items);
pcount = hstoreUniquePairs(pairs, pcount, &buflen);
out = hstorePairs(pairs, pcount, buflen);
}
PG_CATCH();
{
- Py_DECREF(items_v);
+ Py_DECREF(items);
PG_RE_THROW();
}
PG_END_TRY();
diff --git a/contrib/jsonb_plpython/jsonb_plpython.c b/contrib/jsonb_plpython/jsonb_plpython.c
index f44d364c97..1bc984d5c4 100644
--- a/contrib/jsonb_plpython/jsonb_plpython.c
+++ b/contrib/jsonb_plpython/jsonb_plpython.c
@@ -237,17 +237,14 @@ PLyMapping_ToJsonbValue(PyObject *obj, JsonbParseState **jsonb_state)
JsonbValue *out = NULL;
/* We need it volatile, since we use it after longjmp */
- volatile PyObject *items_v = NULL;
+ PyObject *volatile items = NULL;
pcount = PyMapping_Size(obj);
- items_v = PyMapping_Items(obj);
+ items = PyMapping_Items(obj);
PG_TRY();
{
Py_ssize_t i;
- PyObject *items;
-
- items = (PyObject *) items_v;
pushJsonbValue(jsonb_state, WJB_BEGIN_OBJECT, NULL);
@@ -279,7 +276,7 @@ PLyMapping_ToJsonbValue(PyObject *obj, JsonbParseState **jsonb_state)
}
PG_CATCH();
{
- Py_DECREF(items_v);
+ Py_DECREF(items);
PG_RE_THROW();
}
PG_END_TRY();
--
2.21.0
* Re: Fix volatile vs. pointer confusion
@ 2019-03-11 08:31 Michael Paquier <michael@paquier.xyz>
parent: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2 siblings, 1 reply; 6+ messages in thread
From: Michael Paquier @ 2019-03-11 08:31 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: pgsql-hackers
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
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Fix volatile vs. pointer confusion
@ 2019-03-11 11:57 Alvaro Herrera <alvherre@2ndquadrant.com>
parent: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2 siblings, 1 reply; 6+ messages in thread
From: Alvaro Herrera @ 2019-03-11 11:57 UTC (permalink / raw)
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>; +Cc: pgsql-hackers
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
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Fix volatile vs. pointer confusion
@ 2019-03-13 09:13 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 6+ messages in thread
From: Peter Eisentraut @ 2019-03-13 09:13 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers
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
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Fix volatile vs. pointer confusion
@ 2019-03-13 09:14 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
parent: Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 0 replies; 6+ messages in thread
From: Peter Eisentraut @ 2019-03-13 09:14 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@2ndquadrant.com>; +Cc: pgsql-hackers
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
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Fix volatile vs. pointer confusion
@ 2019-03-14 07:48 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
parent: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2 siblings, 0 replies; 6+ messages in thread
From: Peter Eisentraut @ 2019-03-14 07:48 UTC (permalink / raw)
To: pgsql-hackers
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
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2019-03-14 07:48 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-03-11 07:23 Fix volatile vs. pointer confusion Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2019-03-11 08:31 ` Michael Paquier <michael@paquier.xyz>
2019-03-13 09:13 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2019-03-11 11:57 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2019-03-13 09:14 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
2019-03-14 07:48 ` Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox