pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: Jacob Champion <pchampion@vmware.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: Daniel Gustafsson <daniel@yesql.se>
Subject: Re: [PATCH] Make jsonapi usable from libpq
Date: Sat, 26 Jun 2021 15:22:43 -0400
Message-ID: <3122443.1624735363@sss.pgh.pa.us> (raw )
In-Reply-To: <3045186.1624729430@sss.pgh.pa.us >
References: <a250d475ba1c0cc0efb7dfec8e538fcc77cdcb8e.camel@vmware.com >
<YNQedHwheUaTnnGt@paquier.xyz >
<daeb22ec6ca8ef61e94d766a9b35fb03cabed38e.camel@vmware.com >
<YNZ2mnsbDVJQrA/a@paquier.xyz >
<3045186.1624729430@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
Attachments:
avoid-calling-abort-in-fe-print.c-1.patch (text/x-diff, 5.3K) download
diff --git a/src/interfaces/libpq/fe-print.c b/src/interfaces/libpq/fe-print.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 PQprintOpt *po)
unsigned char *fieldNotNum = NULL;
char *border = NULL;
char **fields = NULL;
- const char **fieldNames;
+ const char **fieldNames = NULL;
int fieldMaxLen = 0;
int numFieldName;
int fs_len = strlen(po->fieldSep);
int total_line_length = 0;
- int usePipe = 0;
+ bool usePipe = false;
char *pagerenv;
#if defined(ENABLE_THREAD_SAFETY) && !defined(WIN32)
@@ -111,17 +111,17 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
if (!(fieldNames = (const char **) calloc(nFields, sizeof(char *))))
{
fprintf(stderr, libpq_gettext("out of memory\n"));
- abort();
+ goto exit;
}
if (!(fieldNotNum = (unsigned char *) calloc(nFields, 1)))
{
fprintf(stderr, libpq_gettext("out of memory\n"));
- abort();
+ goto exit;
}
if (!(fieldMax = (int *) calloc(nFields, sizeof(int))))
{
fprintf(stderr, libpq_gettext("out of memory\n"));
- abort();
+ goto exit;
}
for (numFieldName = 0;
po->fieldName && po->fieldName[numFieldName];
@@ -190,7 +190,7 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
fout = popen(pagerenv, "w");
if (fout)
{
- usePipe = 1;
+ usePipe = true;
#ifndef WIN32
#ifdef ENABLE_THREAD_SAFETY
if (pq_block_sigpipe(&osigset, &sigpipe_pending) == 0)
@@ -207,10 +207,11 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
if (!po->expanded && (po->align || po->html3))
{
- if (!(fields = (char **) calloc(nFields * (nTups + 1), sizeof(char *))))
+ if (!(fields = (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 PQprintOpt *po)
fprintf(fout, libpq_gettext("-- RECORD %d --\n"), i);
}
for (j = 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("</table>\n", fout);
}
@@ -297,18 +301,34 @@ PQprint(FILE *fout, const PGresult *res, const PQprintOpt *po)
for (i = 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) == 1) ? "" : "s");
if (po->html3 && !po->expanded)
fputs("</table>\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 = ((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 PQprintOpt *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] = (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 = 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("</tr>", fout);
@@ -609,7 +628,7 @@ PQdisplayTuples(const PGresult *res,
if (!fLength)
{
fprintf(stderr, libpq_gettext("out of memory\n"));
- abort();
+ return;
}
for (j = 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 = 0; i < width; i++)
tborder[i] = '-';
view thread (21+ messages) latest in thread
Message-ID: <3122443.1624735363@sss.pgh.pa.us>
Permalink: ../3122443.1624735363@sss.pgh.pa.us/
Also on: postgresql.org/message-id/3122443.1624735363@sss.pgh.pa.us
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, michael@paquier.xyz, pchampion@vmware.com, daniel@yesql.se
Subject: Re: [PATCH] Make jsonapi usable from libpq
In-Reply-To: <3122443.1624735363@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox