AppendStringInfoChar instead of appendStringInfoString

Started by Hou, Zhijieover 5 years ago3 messages
#1Hou, Zhijie
houzj.fnst@cn.fujitsu.com
1 attachment(s)

Hi

In (/src/backend/replication/backup_manifest.c)

I found the following code:

appendStringInfoString(&buf, "\n");
appendStringInfoString(&buf, "\"");

Since only one bit string is appended here,
I think it will be better to call appendStringInfoChar.

Best reagrds,
houzj

Attachments:

0001-appendStringInfoChar-instead-of-appendStringInfoString.patchapplication/octet-stream; name=0001-appendStringInfoChar-instead-of-appendStringInfoString.patchDownload
From a400dfa64246433f2205da13c477362320db5ac6 Mon Sep 17 00:00:00 2001
From: root <root@localhost.localdomain>
Date: Fri, 25 Sep 2020 12:33:07 -0400
Subject: [PATCH] appendStringInfoChar instead of appendStringInfoString

---
 src/backend/replication/backup_manifest.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/backend/replication/backup_manifest.c b/src/backend/replication/backup_manifest.c
index a43c793..556e6b5 100644
--- a/src/backend/replication/backup_manifest.c
+++ b/src/backend/replication/backup_manifest.c
@@ -112,7 +112,7 @@ AddFileToBackupManifest(backup_manifest_info *manifest, const char *spcoid,
 	initStringInfo(&buf);
 	if (manifest->first_file)
 	{
-		appendStringInfoString(&buf, "\n");
+		appendStringInfoChar(&buf, '\n');
 		manifest->first_file = false;
 	}
 	else
@@ -152,7 +152,7 @@ AddFileToBackupManifest(backup_manifest_info *manifest, const char *spcoid,
 	enlargeStringInfo(&buf, 128);
 	buf.len += pg_strftime(&buf.data[buf.len], 128, "%Y-%m-%d %H:%M:%S %Z",
 						   pg_gmtime(&mtime));
-	appendStringInfoString(&buf, "\"");
+	appendStringInfoChar(&buf, '"');
 
 	/* Add checksum information. */
 	if (checksum_ctx->type != CHECKSUM_TYPE_NONE)
@@ -168,7 +168,7 @@ AddFileToBackupManifest(backup_manifest_info *manifest, const char *spcoid,
 		enlargeStringInfo(&buf, 2 * checksumlen);
 		buf.len += hex_encode((char *) checksumbuf, checksumlen,
 							  &buf.data[buf.len]);
-		appendStringInfoString(&buf, "\"");
+		appendStringInfoChar(&buf, '"');
 	}
 
 	/* Close out the object. */
-- 
1.8.3.1

#2Justin Pryzby
pryzby@telsasoft.com
In reply to: Hou, Zhijie (#1)
Re: AppendStringInfoChar instead of appendStringInfoString

On Fri, Sep 25, 2020 at 08:49:57AM +0000, Hou, Zhijie wrote:

In (/src/backend/replication/backup_manifest.c)

I found the following code:

appendStringInfoString(&buf, "\n");
appendStringInfoString(&buf, "\"");

Good point. There's another one:

$ git grep -E 'appendStringInfoString.*".{,1}");'
src/backend/utils/adt/ruleutils.c: appendStringInfoString(buf, "(");

I noticed you added a similar thread here.
https://commitfest.postgresql.org/30/

I think this one could be combined as a single patchset with the existing CF
entry for the other thread.

/messages/by-id/cb172cf4361e4c7ba7167429070979d4@G08CNEXMBPEKD05.g08.fujitsu.local
/messages/by-id/ce9a8564ccf1435eacf14bb7364ae9de@G08CNEXMBPEKD05.g08.fujitsu.local

--
Justin

#3Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Justin Pryzby (#2)
Re: AppendStringInfoChar instead of appendStringInfoString

On 2020-Sep-27, Justin Pryzby wrote:

On Fri, Sep 25, 2020 at 08:49:57AM +0000, Hou, Zhijie wrote:

In (/src/backend/replication/backup_manifest.c)

I found the following code:

appendStringInfoString(&buf, "\n");
appendStringInfoString(&buf, "\"");

Good point. There's another one:

These have been handled by David Rowley in commit 110d81728a0a.