Hi All, We have (StringInfo::len == 0) checks at many places. I thought it would be better to wrap that into a function isEmptyStringInfo() to make those checks more readable and also abstract the logic to check emptiness of a StringInfo. I think this will be useful to extensions outside core which also have these checks. They won't need to worry about that logic/code being changed in future; rare but not impossible case.
Probably we should have similar support for PQExpBuffer as well, which will be more useful to hide the internals of PQExpBuffer from client code. But I haven't included those changes in this patch. I can do that if hackers like the idea. -- Best Wishes, Ashutosh Bapat
From 0ff57f4b8fffff0094d581806f5553397c286736 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> Date: Wed, 23 Mar 2022 17:40:44 +0530 Subject: [PATCH] isEmptyStringInfo() to check whether StringInfo has no data Add a function isEmptyStringInfo() which returns true if there is no data in the given StringInfo buffer. False otherwise. This hides the logic to check emptiness in stringinfo module. Ashutosh Bapat --- src/backend/catalog/objectaddress.c | 4 ++-- src/backend/catalog/pg_shdepend.c | 2 +- src/backend/commands/copyfromparse.c | 2 +- src/backend/commands/copyto.c | 2 +- src/backend/commands/explain.c | 4 ++-- src/backend/utils/adt/ruleutils.c | 4 ++-- src/backend/utils/fmgr/dfmgr.c | 4 ++-- src/include/lib/stringinfo.h | 10 ++++++++++ 8 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index f30c742d48..5aa97bb485 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -4066,7 +4066,7 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) } /* an empty buffer is equivalent to no object found */ - if (buffer.len == 0) + if (isEmptyStringInfo(&buffer)) return NULL; return buffer.data; @@ -5938,7 +5938,7 @@ getObjectIdentityParts(const ObjectAddress *object, else { /* an empty buffer is equivalent to no object found */ - if (buffer.len == 0) + if (isEmptyStringInfo(&buffer)) { Assert((objname == NULL || *objname == NIL) && (objargs == NULL || *objargs == NIL)); diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c index 3e8fa008b9..597927011c 100644 --- a/src/backend/catalog/pg_shdepend.c +++ b/src/backend/catalog/pg_shdepend.c @@ -810,7 +810,7 @@ checkSharedDependencies(Oid classId, Oid objectId, pfree(objects); list_free_deep(remDeps); - if (descs.len == 0) + if (isEmptyStringInfo(&descs)) { pfree(descs.data); pfree(alldescs.data); diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c index baf328b620..ec4eac0ea6 100644 --- a/src/backend/commands/copyfromparse.c +++ b/src/backend/commands/copyfromparse.c @@ -776,7 +776,7 @@ NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields) * characters, we act as though it was newline followed by EOF, ie, * process the line and then exit loop on next iteration. */ - if (done && cstate->line_buf.len == 0) + if (done && isEmptyStringInfo(&(cstate->line_buf)) return false; /* Parse the line into de-escaped field values */ diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index 55c38b04c4..9e6d094764 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -156,7 +156,7 @@ static void SendCopyEnd(CopyToState cstate) { /* Shouldn't have any unsent data */ - Assert(cstate->fe_msgbuf->len == 0); + Assert(isEmptyStringInfo(&(cstate->fe_msgbuf)); /* Send Copy Done message */ pq_putemptymessage('c'); } diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 9f632285b6..4b9c681c4b 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -4174,7 +4174,7 @@ ExplainOpenWorker(int n, ExplainState *es) */ if (es->format == EXPLAIN_FORMAT_TEXT) { - if (es->str->len == 0) + if (isEmptyStringInfo(es->str)) { ExplainIndentText(es); appendStringInfo(es->str, "Worker %d: ", n); @@ -4870,7 +4870,7 @@ static void ExplainIndentText(ExplainState *es) { Assert(es->format == EXPLAIN_FORMAT_TEXT); - if (es->str->len == 0 || es->str->data[es->str->len - 1] == '\n') + if (isEmptyStringInfo(es->str) || es->str->data[es->str->len - 1] == '\n') appendStringInfoSpaces(es->str, es->indent * 2); } diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index b16526e65e..43028dbda8 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -607,7 +607,7 @@ pg_get_ruledef_worker(Oid ruleoid, int prettyFlags) if (SPI_finish() != SPI_OK_FINISH) elog(ERROR, "SPI_finish failed"); - if (buf.len == 0) + if (isEmptyStringInfo(&buf)) return NULL; return buf.data; @@ -803,7 +803,7 @@ pg_get_viewdef_worker(Oid viewoid, int prettyFlags, int wrapColumn) if (SPI_finish() != SPI_OK_FINISH) elog(ERROR, "SPI_finish failed"); - if (buf.len == 0) + if (isEmptyStringInfo(&buf)) return NULL; return buf.data; diff --git a/src/backend/utils/fmgr/dfmgr.c b/src/backend/utils/fmgr/dfmgr.c index 050da78080..ef4e3420b0 100644 --- a/src/backend/utils/fmgr/dfmgr.c +++ b/src/backend/utils/fmgr/dfmgr.c @@ -382,7 +382,7 @@ incompatible_module_error(const char *libname, } if (module_magic_data->float8byval != magic_data.float8byval) { - if (details.len) + if (!isEmptyStringInfo(&details)) appendStringInfoChar(&details, '\n'); appendStringInfo(&details, _("Server has FLOAT8PASSBYVAL = %s, library has %s."), @@ -390,7 +390,7 @@ incompatible_module_error(const char *libname, module_magic_data->float8byval ? "true" : "false"); } - if (details.len == 0) + if (isEmptyStringInfo(&details)) appendStringInfoString(&details, _("Magic block has unexpected length or padding difference.")); diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h index 9b755c4883..0e659b976a 100644 --- a/src/include/lib/stringinfo.h +++ b/src/include/lib/stringinfo.h @@ -158,4 +158,14 @@ extern void appendBinaryStringInfoNT(StringInfo str, */ extern void enlargeStringInfo(StringInfo str, int needed); +/*------------------------ + * isEmptyStringInfo + * Returns true if the given StringInfo does not have any data in it. False + * otherwise. + */ +static inline isEmptyStringInfo(StringInfo str) +{ + return str->len == 0; +} + #endif /* STRINGINFO_H */ -- 2.25.1