On Sat, Aug 27, 2016 at 12:33 AM, Aleksander Alekseev <a.aleks...@postgrespro.ru> wrote: > Your patch [1] was marked as "Needs review" so I decided to take a look.
Thanks for the input! > It looks good to me. However I found dozens of places in PostgreSQL code > that seem to have similar problem you are trying to fix [2]. As far as I > understand these are only places left that don't check malloc/realloc/ > strdup return values properly. I thought maybe you will be willing to > fix they too so we could forget about this problem forever. So my lookup was still incomplete. > If not I will be happy to do it. However in this case we need someone > familiar with affected parts of the code who will be willing to re-check > a new patch since I'm not filling particularly confident about how > exactly errors should be handled in all these cases. I'll do it and compress that in my patch. > By the way maybe someone knows other procedures besides malloc, realloc > and strdup that require special attention? I don't know how you did it, but this email has visibly broken the original thread. Did you change the topic name? ./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. ./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(). ./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. ./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. ./src/backend/utils/mmgr/mcxt.c: node = (MemoryContext) malloc(needed); You cannot do much here... ./src/timezone/zic.c: lcltime = strdup(optarg); This is upstream code, not worth worrying. ./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. ./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. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers