On Sat, Aug 27, 2016 at 10:24 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> ./src/backend/postmaster/postmaster.c: userDoption =
> strdup(optarg);
> [...]
> ./src/backend/bootstrap/bootstrap.c: userDoption =
> strdup(optarg);
> [...]
> ./src/backend/tcop/postgres.c: userDoption = strdup(optarg);
> We cannot use pstrdup here because memory contexts are not set up
> here, still it would be better than crashing, but I am not sure if
> that's worth complicating this code.. Other opinions are welcome.
Those are not changed. We could have some NULL-checks but I am not
sure that's worth the complication here.
> ./contrib/vacuumlo/vacuumlo.c: param.pg_user = strdup(optarg);
> [...]
> ./contrib/pg_standby/pg_standby.c: triggerPath = strdup(optarg);
> [...]
> ./src/bin/pg_archivecleanup/pg_archivecleanup.c:
> additional_ext = strdup(optarg); /* Extension to remove
> Right we can do something here with pstrdup().
Those are updated with pg_strdup().
> ./contrib/spi/refint.c: plan->splan = (SPIPlanPtr *)
> malloc(sizeof(SPIPlanPtr));
> Regarding refint.c, you can see upthread. Instead of reworking the
> code it would be better to just drop it from the tree.
I'd rather see this nuked out of the surface of the code tree.
> ./src/backend/utils/adt/pg_locale.c: grouping = strdup(extlconv->grouping);
> Here that would be a failure with an elog() as this is getting used by
> things like NUM_TOCHAR_finish and PGLC_localeconv.
Done.
> ./src/pl/tcl/pltcl.c: prodesc->user_proname =
> strdup(NameStr(procStruct->proname));
> ./src/pl/tcl/pltcl.c: prodesc->internal_proname =
> strdup(internal_proname);
> ./src/pl/tcl/pltcl.c- if (prodesc->user_proname == NULL ||
> prodesc->internal_proname == NULL)
> ./src/pl/tcl/pltcl.c- ereport(ERROR,
> ./src/pl/tcl/pltcl.c- (errcode(ERRCODE_OUT_OF_MEMORY),
> ./src/pl/tcl/pltcl.c- errmsg("out of memory")));
> Ah, yes. Here we need to do a free(prodesc) first.
Done.
> ./src/common/exec.c: putenv(strdup(env_path));
> set_pglocale_pgservice() is used at the beginning of each process run,
> meaning that a failure here would be printf(stderr), followed by
> exit() for frontend, even ECPG as this compiles with -DFRONTEND.
> Backend can use elog(ERROR) btw.
Done.
Regarding the handling of strdup in libpq the code is careful in its
handling after a review, so we had better do nothing.
After that, there are two calls to realloc and one call to malloc that
deserve some attention, though those happen in pgc.l and I am not
exactly sure how to handle them.
As Alexander's email
(https://www.postgresql.org/message-id/20160826153358.GA29981%40e733)
has broken this thread, I am attaching to this thread the analysis
report that has been generated by Alexander previously. It was
referenced in only an URL.
Attached is an updated patch addressing those extra points.
--
Michael
