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]> 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 05:04 Michael Paquier <[email protected]> parent: Kyotaro Horiguchi <[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 12:28 Ranier Vilela <[email protected]> parent: Kyotaro Horiguchi <[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
* Re: Fix use of possible uninitialized variable retval (src/pl/plpgsql/src/pl_handler.c) @ 2024-06-05 12:34 Ranier Vilela <[email protected]> parent: Michael Paquier <[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 14:27 Julien Rouhaud <[email protected]> parent: 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-06 04:29 Michael Paquier <[email protected]> parent: Julien Rouhaud <[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
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