public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 1/5] bootstrap: convert Typ to a List*
7+ messages / 5 participants
[nested] [flat]
* [PATCH 1/5] bootstrap: convert Typ to a List*
@ 2020-11-20 02:48 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Justin Pryzby @ 2020-11-20 02:48 UTC (permalink / raw)
---
src/backend/bootstrap/bootstrap.c | 69 ++++++++++++++-----------------
1 file changed, 31 insertions(+), 38 deletions(-)
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 6f615e6622..18eb62ca47 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -159,7 +159,7 @@ struct typmap
FormData_pg_type am_typ;
};
-static struct typmap **Typ = NULL;
+static List *Typ = NIL; /* List of struct typmap* */
static struct typmap *Ap = NULL;
static Datum values[MAXATTR]; /* current row's attribute values */
@@ -597,7 +597,7 @@ boot_openrel(char *relname)
* pg_type must be filled before any OPEN command is executed, hence we
* can now populate the Typ array if we haven't yet.
*/
- if (Typ == NULL)
+ if (Typ == NIL)
populate_typ_array();
if (boot_reldesc != NULL)
@@ -688,7 +688,7 @@ DefineAttr(char *name, char *type, int attnum, int nullness)
typeoid = gettype(type);
- if (Typ != NULL)
+ if (Typ != NIL)
{
attrtypes[attnum]->atttypid = Ap->am_oid;
attrtypes[attnum]->attlen = Ap->am_typ.typlen;
@@ -877,36 +877,25 @@ populate_typ_array(void)
Relation rel;
TableScanDesc scan;
HeapTuple tup;
- int nalloc;
- int i;
-
- Assert(Typ == NULL);
- nalloc = 512;
- Typ = (struct typmap **)
- MemoryContextAlloc(TopMemoryContext, nalloc * sizeof(struct typmap *));
+ Assert(Typ == NIL);
rel = table_open(TypeRelationId, NoLock);
scan = table_beginscan_catalog(rel, 0, NULL);
- i = 0;
while ((tup = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
Form_pg_type typForm = (Form_pg_type) GETSTRUCT(tup);
+ struct typmap *newtyp;
+ MemoryContext old;
- /* make sure there will be room for a trailing NULL pointer */
- if (i >= nalloc - 1)
- {
- nalloc *= 2;
- Typ = (struct typmap **)
- repalloc(Typ, nalloc * sizeof(struct typmap *));
- }
- Typ[i] = (struct typmap *)
- MemoryContextAlloc(TopMemoryContext, sizeof(struct typmap));
- Typ[i]->am_oid = typForm->oid;
- memcpy(&(Typ[i]->am_typ), typForm, sizeof(Typ[i]->am_typ));
- i++;
+ old = MemoryContextSwitchTo(TopMemoryContext);
+ newtyp = (struct typmap *) palloc(sizeof(struct typmap));
+ Typ = lappend(Typ, newtyp);
+ MemoryContextSwitchTo(old);
+
+ newtyp->am_oid = typForm->oid;
+ memcpy(&newtyp->am_typ, typForm, sizeof(newtyp->am_typ));
}
- Typ[i] = NULL; /* Fill trailing NULL pointer */
table_endscan(scan);
table_close(rel, NoLock);
}
@@ -925,16 +914,17 @@ populate_typ_array(void)
static Oid
gettype(char *type)
{
- if (Typ != NULL)
+ if (Typ != NIL)
{
- struct typmap **app;
+ ListCell *lc;
- for (app = Typ; *app != NULL; app++)
+ foreach (lc, Typ)
{
- if (strncmp(NameStr((*app)->am_typ.typname), type, NAMEDATALEN) == 0)
+ struct typmap *app = lfirst(lc);
+ if (strncmp(NameStr(app->am_typ.typname), type, NAMEDATALEN) == 0)
{
- Ap = *app;
- return (*app)->am_oid;
+ Ap = app;
+ return app->am_oid;
}
}
}
@@ -980,14 +970,17 @@ boot_get_type_io_data(Oid typid,
if (Typ != NULL)
{
/* We have the boot-time contents of pg_type, so use it */
- struct typmap **app;
- struct typmap *ap;
-
- app = Typ;
- while (*app && (*app)->am_oid != typid)
- ++app;
- ap = *app;
- if (ap == NULL)
+ struct typmap *ap = NULL;
+ ListCell *lc;
+
+ foreach (lc, Typ)
+ {
+ ap = lfirst(lc);
+ if (ap->am_oid == typid)
+ break;
+ }
+
+ if (!ap || ap->am_oid != typid)
elog(ERROR, "type OID %u not found in Typ list", typid);
*typlen = ap->am_typ.typlen;
--
2.26.2
--------------614DDB87AFFED893713AC0E9
Content-Type: text/x-patch; charset=UTF-8;
name="0002-Allow-composite-types-in-bootstrap-20210304.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="0002-Allow-composite-types-in-bootstrap-20210304.patch"
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c)
@ 2024-06-05 04:12 Kyotaro Horiguchi <[email protected]>
2024-06-05 05:04 ` Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Michael Paquier <[email protected]>
2024-06-05 12:28 ` Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Ranier Vilela <[email protected]>
0 siblings, 2 replies; 7+ messages in thread
From: Kyotaro Horiguchi @ 2024-06-05 04:12 UTC (permalink / raw)
To: [email protected]; +Cc: pgsql-hackers
At Mon, 27 May 2024 11:31:24 -0300, Ranier Vilela <[email protected]> wrote in
> Hi.
>
> The function *plpgsql_inline_handler* can use uninitialized
> variable retval, if PG_TRY fails.
> Fix like function*plpgsql_call_handler* wich declare retval as
> volatile and initialize to (Datum 0).
If PG_TRY fails, retval is not actually accessed, so no real issue
exists. Commit 7292fd8f1c changed plpgsql_call_handler() to the
current form, but as stated in its commit message, it did not fix a
real issue and was solely to silence compiler.
I believe we do not need to modify plpgsql_inline_handler() unless
compiler actually issues a false warning for it.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c)
2024-06-05 04:12 Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Kyotaro Horiguchi <[email protected]>
@ 2024-06-05 05:04 ` Michael Paquier <[email protected]>
2024-06-05 12:34 ` Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Ranier Vilela <[email protected]>
2024-06-05 14:27 ` Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Julien Rouhaud <[email protected]>
1 sibling, 2 replies; 7+ messages in thread
From: Michael Paquier @ 2024-06-05 05:04 UTC (permalink / raw)
To: Kyotaro Horiguchi <[email protected]>; +Cc: [email protected]; pgsql-hackers
On Wed, Jun 05, 2024 at 01:12:41PM +0900, Kyotaro Horiguchi wrote:
> At Mon, 27 May 2024 11:31:24 -0300, Ranier Vilela <[email protected]> wrote in
>> The function *plpgsql_inline_handler* can use uninitialized
>> variable retval, if PG_TRY fails.
>> Fix like function*plpgsql_call_handler* wich declare retval as
>> volatile and initialize to (Datum 0).
You forgot to read elog.h, explaining under which circumstances
variables related to TRY/CATCH block should be marked as volatile.
There is a big "Note:" paragraph.
It is not the first time that this is mentioned on this list: but
sending a report without looking at the reason why a change is
justified makes everybody waste time. That's not productive.
> If PG_TRY fails, retval is not actually accessed, so no real issue
> exists. Commit 7292fd8f1c changed plpgsql_call_handler() to the
> current form, but as stated in its commit message, it did not fix a
> real issue and was solely to silence compiler.
This complain was from lapwing, that uses a version of gcc which
produces a lot of noise with incorrect issues. It is one of the only
32b buildfarm members, so it still has a lot of value.
> I believe we do not need to modify plpgsql_inline_handler() unless
> compiler actually issues a false warning for it.
If we were to do something, that would be to remove the volatile from
plpgsql_call_handler() at the end once we don't have in the buildfarm
compilers that complain about it, because there is no reason to use a
volatile in this case. :)
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c)
2024-06-05 04:12 Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Kyotaro Horiguchi <[email protected]>
2024-06-05 05:04 ` Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Michael Paquier <[email protected]>
@ 2024-06-05 12:34 ` Ranier Vilela <[email protected]>
1 sibling, 0 replies; 7+ messages in thread
From: Ranier Vilela @ 2024-06-05 12:34 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; pgsql-hackers
Em qua., 5 de jun. de 2024 às 02:04, Michael Paquier <[email protected]>
escreveu:
> On Wed, Jun 05, 2024 at 01:12:41PM +0900, Kyotaro Horiguchi wrote:
> > At Mon, 27 May 2024 11:31:24 -0300, Ranier Vilela <[email protected]>
> wrote in
> >> The function *plpgsql_inline_handler* can use uninitialized
> >> variable retval, if PG_TRY fails.
> >> Fix like function*plpgsql_call_handler* wich declare retval as
> >> volatile and initialize to (Datum 0).
>
> You forgot to read elog.h, explaining under which circumstances
> variables related to TRY/CATCH block should be marked as volatile.
> There is a big "Note:" paragraph.
>
> It is not the first time that this is mentioned on this list: but
> sending a report without looking at the reason why a change is
> justified makes everybody waste time. That's not productive.
>
Of course, this is very bad when it happens.
>
> > If PG_TRY fails, retval is not actually accessed, so no real issue
> > exists. Commit 7292fd8f1c changed plpgsql_call_handler() to the
> > current form, but as stated in its commit message, it did not fix a
> > real issue and was solely to silence compiler.
>
> This complain was from lapwing, that uses a version of gcc which
> produces a lot of noise with incorrect issues. It is one of the only
> 32b buildfarm members, so it still has a lot of value.
>
I posted the report, because of an uninitialized variable warning.
Which is one of the most problematic situations, when it *actually exists*.
> > I believe we do not need to modify plpgsql_inline_handler() unless
> > compiler actually issues a false warning for it.
>
> If we were to do something, that would be to remove the volatile from
> plpgsql_call_handler() at the end once we don't have in the buildfarm
> compilers that complain about it, because there is no reason to use a
> volatile in this case. :)
>
I don't see any motivation, since there are no reports.
best regards,
Ranier Vilela
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c)
2024-06-05 04:12 Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Kyotaro Horiguchi <[email protected]>
2024-06-05 05:04 ` Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Michael Paquier <[email protected]>
@ 2024-06-05 14:27 ` Julien Rouhaud <[email protected]>
2024-06-06 04:29 ` Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Michael Paquier <[email protected]>
1 sibling, 1 reply; 7+ messages in thread
From: Julien Rouhaud @ 2024-06-05 14:27 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; [email protected]; pgsql-hackers
On Wed, Jun 5, 2024 at 1:05 PM Michael Paquier <[email protected]> wrote:
>
> This complain was from lapwing, that uses a version of gcc which
> produces a lot of noise with incorrect issues. It is one of the only
> 32b buildfarm members, so it still has a lot of value.
Note that I removed the -Werror from lapwing a long time ago, so at
least this animal shouldn't lead to hackish fixes for false positive
anymore.
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c)
2024-06-05 04:12 Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Kyotaro Horiguchi <[email protected]>
2024-06-05 05:04 ` Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Michael Paquier <[email protected]>
2024-06-05 14:27 ` Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Julien Rouhaud <[email protected]>
@ 2024-06-06 04:29 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Michael Paquier @ 2024-06-06 04:29 UTC (permalink / raw)
To: Julien Rouhaud <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; [email protected]; pgsql-hackers
On Wed, Jun 05, 2024 at 10:27:51PM +0800, Julien Rouhaud wrote:
> Note that I removed the -Werror from lapwing a long time ago, so at
> least this animal shouldn't lead to hackish fixes for false positive
> anymore.
That's good to know. Thanks for the update.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c)
2024-06-05 04:12 Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Kyotaro Horiguchi <[email protected]>
@ 2024-06-05 12:28 ` Ranier Vilela <[email protected]>
1 sibling, 0 replies; 7+ messages in thread
From: Ranier Vilela @ 2024-06-05 12:28 UTC (permalink / raw)
To: Kyotaro Horiguchi <[email protected]>; +Cc: pgsql-hackers
Em qua., 5 de jun. de 2024 às 01:12, Kyotaro Horiguchi <
[email protected]> escreveu:
> At Mon, 27 May 2024 11:31:24 -0300, Ranier Vilela <[email protected]>
> wrote in
> > Hi.
> >
> > The function *plpgsql_inline_handler* can use uninitialized
> > variable retval, if PG_TRY fails.
> > Fix like function*plpgsql_call_handler* wich declare retval as
> > volatile and initialize to (Datum 0).
>
> If PG_TRY fails, retval is not actually accessed, so no real issue
> exists.
You say it for this call
PG_RE_THROW();
> Commit 7292fd8f1c changed plpgsql_call_handler() to the
> current form, but as stated in its commit message, it did not fix a
> real issue and was solely to silence compiler.
>
> I believe we do not need to modify plpgsql_inline_handler() unless
> compiler actually issues a false warning for it.
>
Yeah, there is a warning, but not from the compiler.
best regards,
Ranier Vilela
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2024-06-06 04:29 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20 02:48 [PATCH 1/5] bootstrap: convert Typ to a List* Justin Pryzby <[email protected]>
2024-06-05 04:12 Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Kyotaro Horiguchi <[email protected]>
2024-06-05 05:04 ` Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Michael Paquier <[email protected]>
2024-06-05 12:34 ` Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Ranier Vilela <[email protected]>
2024-06-05 14:27 ` Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Julien Rouhaud <[email protected]>
2024-06-06 04:29 ` Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Michael Paquier <[email protected]>
2024-06-05 12:28 ` Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) Ranier Vilela <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox