Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lxDtJ-0005zZ-5Z for pgsql-hackers@arkaria.postgresql.org; Sat, 26 Jun 2021 19:22:57 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1lxDtF-0003Rm-En for pgsql-hackers@arkaria.postgresql.org; Sat, 26 Jun 2021 19:22:53 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lxDtE-0003Re-V1 for pgsql-hackers@lists.postgresql.org; Sat, 26 Jun 2021 19:22:53 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lxDt8-0000ev-8A for pgsql-hackers@postgresql.org; Sat, 26 Jun 2021 19:22:51 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 15QJMhff3122444; Sat, 26 Jun 2021 15:22:43 -0400 From: Tom Lane To: Michael Paquier cc: Jacob Champion , "pgsql-hackers@postgresql.org" , Daniel Gustafsson Subject: Re: [PATCH] Make jsonapi usable from libpq In-reply-to: <3045186.1624729430@sss.pgh.pa.us> References: <3045186.1624729430@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Sat, 26 Jun 2021 13:43:50 -0400" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <3122402.1624735313.0@sss.pgh.pa.us> Date: Sat, 26 Jun 2021 15:22:43 -0400 Message-ID: <3122443.1624735363@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <3122402.1624735313.1@sss.pgh.pa.us> I wrote: > I spent some time looking for other undesirable symbol dependencies > in libpq, and soon found a couple. PGTHREAD_ERROR potentially calls > abort(), which seems even worse than exit-on-OOM, although I don't > think we've ever heard a report of that being hit. Also, > fe-print.c's handling of OOM isn't nice at all: > fprintf(stderr, libpq_gettext("out of memory\n")); > abort(); > Although fe-print.c is semi-deprecated, it still seems like it'd > be a good idea to clean that up. fe-print.c seems easy enough to clean up, as per attached. Not real sure what to do about PGTHREAD_ERROR. regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="avoid-calling-abort-in-fe-print.c-1.patch"; charset="us-ascii" Content-ID: <3122402.1624735313.2@sss.pgh.pa.us> Content-Description: avoid-calling-abort-in-fe-print.c-1.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/interfaces/libpq/fe-print.c b/src/interfaces/libpq/fe-pri= nt.c index fc7d84844e..0831950b12 100644 --- a/src/interfaces/libpq/fe-print.c +++ b/src/interfaces/libpq/fe-print.c @@ -37,7 +37,7 @@ #include "libpq-int.h" = = -static void do_field(const PQprintOpt *po, const PGresult *res, +static bool do_field(const PQprintOpt *po, const PGresult *res, const int i, const int j, const int fs_len, char **fields, const int nFields, const char **fieldNames, @@ -80,12 +80,12 @@ PQprint(FILE *fout, const PGresult *res, const PQprint= Opt *po) unsigned char *fieldNotNum =3D NULL; char *border =3D NULL; char **fields =3D NULL; - const char **fieldNames; + const char **fieldNames =3D NULL; int fieldMaxLen =3D 0; int numFieldName; int fs_len =3D strlen(po->fieldSep); int total_line_length =3D 0; - int usePipe =3D 0; + bool usePipe =3D false; char *pagerenv; = #if defined(ENABLE_THREAD_SAFETY) && !defined(WIN32) @@ -111,17 +111,17 @@ PQprint(FILE *fout, const PGresult *res, const PQpri= ntOpt *po) if (!(fieldNames =3D (const char **) calloc(nFields, sizeof(char *)))) { fprintf(stderr, libpq_gettext("out of memory\n")); - abort(); + goto exit; } if (!(fieldNotNum =3D (unsigned char *) calloc(nFields, 1))) { fprintf(stderr, libpq_gettext("out of memory\n")); - abort(); + goto exit; } if (!(fieldMax =3D (int *) calloc(nFields, sizeof(int)))) { fprintf(stderr, libpq_gettext("out of memory\n")); - abort(); + goto exit; } for (numFieldName =3D 0; po->fieldName && po->fieldName[numFieldName]; @@ -190,7 +190,7 @@ PQprint(FILE *fout, const PGresult *res, const PQprint= Opt *po) fout =3D popen(pagerenv, "w"); if (fout) { - usePipe =3D 1; + usePipe =3D true; #ifndef WIN32 #ifdef ENABLE_THREAD_SAFETY if (pq_block_sigpipe(&osigset, &sigpipe_pending) =3D=3D 0) @@ -207,10 +207,11 @@ PQprint(FILE *fout, const PGresult *res, const PQpri= ntOpt *po) = if (!po->expanded && (po->align || po->html3)) { - if (!(fields =3D (char **) calloc(nFields * (nTups + 1), sizeof(char *= )))) + if (!(fields =3D (char **) calloc((size_t) nTups + 1, + nFields * sizeof(char *)))) { fprintf(stderr, libpq_gettext("out of memory\n")); - abort(); + goto exit; } } else if (po->header && !po->html3) @@ -264,9 +265,12 @@ PQprint(FILE *fout, const PGresult *res, const PQprin= tOpt *po) fprintf(fout, libpq_gettext("-- RECORD %d --\n"), i); } for (j =3D 0; j < nFields; j++) - do_field(po, res, i, j, fs_len, fields, nFields, - fieldNames, fieldNotNum, - fieldMax, fieldMaxLen, fout); + { + if (!do_field(po, res, i, j, fs_len, fields, nFields, + fieldNames, fieldNotNum, + fieldMax, fieldMaxLen, fout)) + goto exit; + } if (po->html3 && po->expanded) fputs("\n", fout); } @@ -297,18 +301,34 @@ PQprint(FILE *fout, const PGresult *res, const PQpri= ntOpt *po) for (i =3D 0; i < nTups; i++) output_row(fout, po, nFields, fields, fieldNotNum, fieldMax, border, i); - free(fields); - if (border) - free(border); } if (po->header && !po->html3) fprintf(fout, "(%d row%s)\n\n", PQntuples(res), (PQntuples(res) =3D=3D 1) ? "" : "s"); if (po->html3 && !po->expanded) fputs("\n", fout); - free(fieldMax); - free(fieldNotNum); - free((void *) fieldNames); + +exit: + if (fieldMax) + free(fieldMax); + if (fieldNotNum) + free(fieldNotNum); + if (border) + free(border); + if (fields) + { + /* if calloc succeeded, this shouldn't overflow size_t */ + size_t numfields =3D ((size_t) nTups + 1) * (size_t) nFields; + + while (numfields-- > 0) + { + if (fields[numfields]) + free(fields[numfields]); + } + free(fields); + } + if (fieldNames) + free((void *) fieldNames); if (usePipe) { #ifdef WIN32 @@ -329,7 +349,7 @@ PQprint(FILE *fout, const PGresult *res, const PQprint= Opt *po) } = = -static void +static bool do_field(const PQprintOpt *po, const PGresult *res, const int i, const int j, const int fs_len, char **fields, @@ -397,7 +417,7 @@ do_field(const PQprintOpt *po, const PGresult *res, if (!(fields[i * nFields + j] =3D (char *) malloc(plen + 1))) { fprintf(stderr, libpq_gettext("out of memory\n")); - abort(); + return false; } strcpy(fields[i * nFields + j], pval); } @@ -440,6 +460,7 @@ do_field(const PQprintOpt *po, const PGresult *res, } } } + return true; } = = @@ -467,7 +488,7 @@ do_header(FILE *fout, const PQprintOpt *po, const int = nFields, int *fieldMax, if (!border) { fprintf(stderr, libpq_gettext("out of memory\n")); - abort(); + return NULL; } p =3D border; if (po->standard) @@ -558,8 +579,6 @@ output_row(FILE *fout, const PQprintOpt *po, const int= nFields, char **fields, if (po->standard || field_index + 1 < nFields) fputs(po->fieldSep, fout); } - if (p) - free(p); } if (po->html3) fputs("", fout); @@ -609,7 +628,7 @@ PQdisplayTuples(const PGresult *res, if (!fLength) { fprintf(stderr, libpq_gettext("out of memory\n")); - abort(); + return; } = for (j =3D 0; j < nFields; j++) @@ -707,7 +726,7 @@ PQprintTuples(const PGresult *res, if (!tborder) { fprintf(stderr, libpq_gettext("out of memory\n")); - abort(); + return; } for (i =3D 0; i < width; i++) tborder[i] =3D '-'; ------- =_aaaaaaaaaa0--