Fix an assortment of improper usages of string functions

In a similar effort to f736e18 and 110d817, fixup various usages of string functions where a more appropriate function is available and more fit for purpose. These changes include: 1. Use cstring_to_text_with_len() instead of cstring_to_text() when working with a StringInfoData and the length can easily be obtained. 2. Use appendStringInfoString() instead of appendStringInfo() when no formatting is required. 3. Use pstrdup(...) instead of psprintf("%s", ...) 4. Use pstrdup(...) instead of psprintf(...) (with no formatting) 5. Use appendPQExpBufferChar() instead of appendPQExpBufferStr() when the length of the string being appended is 1. 6. appendStringInfoChar() instead of appendStringInfo() when no formatting is required and string is 1 char long. 7. Use appendPQExpBufferStr(b, .) instead of appendPQExpBuffer(b, "%s", .) 8. Don't use pstrdup when it's fine to just point to the string constant. I (David) did find other cases of #8 but opted to use #4 instead as I wasn't certain enough that applying #8 was ok (e.g in hba.c) Author: Ranier Vilela, David Rowley Discussion: https://postgr.es/m/CAApHDvo2j2+RJBGhNtUz6BxabWWh2Jx16wMUMWKUjv70Ver1vg@mail.gmail.com
postgres · Sep 6, 2022 · 8b26769 · 8b26769
1 parent 6bcda4a
commit 8b26769
Show file tree

Hide file tree

Showing 18 changed files with 277 additions and 284 deletions.
diff --git a/contrib/hstore/hstore_io.c b/contrib/hstore/hstore_io.c
@@ -1325,7 +1325,7 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
 	}
 	appendStringInfoChar(&dst, '}');
 
-	PG_RETURN_TEXT_P(cstring_to_text(dst.data));
+	PG_RETURN_TEXT_P(cstring_to_text_with_len(dst.data, dst.len));
 }
 
 PG_FUNCTION_INFO_V1(hstore_to_json);
@@ -1370,7 +1370,7 @@ hstore_to_json(PG_FUNCTION_ARGS)
 	}
 	appendStringInfoChar(&dst, '}');
 
-	PG_RETURN_TEXT_P(cstring_to_text(dst.data));
+	PG_RETURN_TEXT_P(cstring_to_text_with_len(dst.data, dst.len));
 }
 
 PG_FUNCTION_INFO_V1(hstore_to_jsonb);

diff --git a/contrib/sepgsql/schema.c b/contrib/sepgsql/schema.c
@@ -84,7 +84,7 @@ sepgsql_schema_post_create(Oid namespaceId)
 	 * check db_schema:{create}
 	 */
 	initStringInfo(&audit_name);
-	appendStringInfo(&audit_name, "%s", quote_identifier(nsp_name));
+	appendStringInfoString(&audit_name, quote_identifier(nsp_name));
 	sepgsql_avc_check_perms_label(ncontext,
 								  SEPG_CLASS_DB_SCHEMA,
 								  SEPG_DB_SCHEMA__CREATE,

diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c
@@ -3063,7 +3063,7 @@ brin_minmax_multi_summary_out(PG_FUNCTION_ARGS)
 
 		appendStringInfo(&str, "%s ... %s", a, b);
 
-		c = cstring_to_text(str.data);
+		c = cstring_to_text_with_len(str.data, str.len);
 
 		astate_values = accumArrayResult(astate_values,
 										 PointerGetDatum(c),
@@ -3095,15 +3095,9 @@ brin_minmax_multi_summary_out(PG_FUNCTION_ARGS)
 	{
 		Datum		a;
 		text	   *b;
-		StringInfoData str;
-
-		initStringInfo(&str);
 
 		a = FunctionCall1(&fmgrinfo, ranges_deserialized->values[idx++]);
-
-		appendStringInfoString(&str, DatumGetCString(a));
-
-		b = cstring_to_text(str.data);
+		b = cstring_to_text(DatumGetCString(a));
 
 		astate_values = accumArrayResult(astate_values,
 										 PointerGetDatum(b),

diff --git a/src/backend/access/rmgrdesc/xlogdesc.c b/src/backend/access/rmgrdesc/xlogdesc.c
@@ -319,9 +319,9 @@ XLogRecGetBlockRefInfo(XLogReaderState *record, bool pretty,
 					*fpi_len += XLogRecGetBlock(record, block_id)->bimg_len;
 
 				if (XLogRecBlockImageApply(record, block_id))
-					appendStringInfo(buf, " FPW");
+					appendStringInfoString(buf, " FPW");
 				else
-					appendStringInfo(buf, " FPW for WAL verification");
+					appendStringInfoString(buf, " FPW for WAL verification");
 			}
 		}
 	}

diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c
@@ -514,7 +514,7 @@ llvm_function_reference(LLVMJitContext *context,
 	else if (basename != NULL)
 	{
 		/* internal function */
-		funcname = psprintf("%s", basename);
+		funcname = pstrdup(basename);
 	}
 	else
 	{

diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
@@ -899,7 +899,7 @@ do { \
 				 errmsg("missing entry at end of line"), \
 				 errcontext("line %d of configuration file \"%s\"", \
 							line_num, IdentFileName))); \
-		*err_msg = psprintf("missing entry at end of line"); \
+		*err_msg = pstrdup("missing entry at end of line"); \
 		return NULL; \
 	} \
 } while (0)
@@ -912,7 +912,7 @@ do { \
 				 errmsg("multiple values in ident field"), \
 				 errcontext("line %d of configuration file \"%s\"", \
 							line_num, IdentFileName))); \
-		*err_msg = psprintf("multiple values in ident field"); \
+		*err_msg = pstrdup("multiple values in ident field"); \
 		return NULL; \
 	} \
 } while (0)

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
@@ -4457,7 +4457,7 @@ BackendInitialize(Port *port)
 	appendStringInfo(&ps_data, "%s ", port->user_name);
 	if (!am_walsender)
 		appendStringInfo(&ps_data, "%s ", port->database_name);
-	appendStringInfo(&ps_data, "%s", port->remote_host);
+	appendStringInfoString(&ps_data, port->remote_host);
 	if (port->remote_port[0] != '\0')
 		appendStringInfo(&ps_data, "(%s)", port->remote_port);
 

diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
@@ -769,7 +769,7 @@ fetch_remote_table_info(char *nspname, char *relname,
 		foreach(lc, MySubscription->publications)
 		{
 			if (foreach_current_index(lc) > 0)
-				appendStringInfo(&pub_names, ", ");
+				appendStringInfoString(&pub_names, ", ");
 			appendStringInfoString(&pub_names, quote_literal_cstr(strVal(lfirst(lc))));
 		}
 
@@ -1062,7 +1062,7 @@ copy_table(Relation rel)
 			appendStringInfoString(&cmd, quote_identifier(lrel.attnames[i]));
 		}
 
-		appendStringInfo(&cmd, ") TO STDOUT");
+		appendStringInfoString(&cmd, ") TO STDOUT");
 	}
 	else
 	{

diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
@@ -96,7 +96,7 @@ anytime_typmodout(bool istz, int32 typmod)
 	if (typmod >= 0)
 		return psprintf("(%d)%s", (int) typmod, tz);
 	else
-		return psprintf("%s", tz);
+		return pstrdup(tz);
 }
 
 

diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
@@ -219,7 +219,7 @@ pg_tablespace_databases(PG_FUNCTION_ARGS)
 	}
 
 	if (tablespaceOid == DEFAULTTABLESPACE_OID)
-		location = psprintf("base");
+		location = "base";
 	else
 		location = psprintf("pg_tblspc/%u/%s", tablespaceOid,
 							TABLESPACE_VERSION_DIRECTORY);

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
@@ -1453,7 +1453,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
 		appendStringInfoChar(&buf, ')');
 
 		if (idxrec->indnullsnotdistinct)
-			appendStringInfo(&buf, " NULLS NOT DISTINCT");
+			appendStringInfoString(&buf, " NULLS NOT DISTINCT");
 
 		/*
 		 * If it has options, append "WITH (options)"
@@ -2332,9 +2332,9 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
 									  Anum_pg_constraint_confdelsetcols, &isnull);
 				if (!isnull)
 				{
-					appendStringInfo(&buf, " (");
+					appendStringInfoString(&buf, " (");
 					decompile_column_index_array(val, conForm->conrelid, &buf);
-					appendStringInfo(&buf, ")");
+					appendStringInfoChar(&buf, ')');
 				}
 
 				break;
@@ -2363,7 +2363,7 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
 					((Form_pg_index) GETSTRUCT(indtup))->indnullsnotdistinct)
 					appendStringInfoString(&buf, "NULLS NOT DISTINCT ");
 
-				appendStringInfoString(&buf, "(");
+				appendStringInfoChar(&buf, '(');
 
 				/* Fetch and build target column list */
 				val = SysCacheGetAttr(CONSTROID, tup,
@@ -3583,7 +3583,7 @@ pg_get_function_sqlbody(PG_FUNCTION_ARGS)
 
 	ReleaseSysCache(proctup);
 
-	PG_RETURN_TEXT_P(cstring_to_text(buf.data));
+	PG_RETURN_TEXT_P(cstring_to_text_with_len(buf.data, buf.len));
 }
 
 

diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
@@ -131,7 +131,7 @@ anytimestamp_typmodout(bool istz, int32 typmod)
 	if (typmod >= 0)
 		return psprintf("(%d)%s", (int) typmod, tz);
 	else
-		return psprintf("%s", tz);
+		return pstrdup(tz);
 }
 
 

diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.c
@@ -1509,7 +1509,7 @@ append_db_pattern_cte(PQExpBuffer buf, const PatternInfoArray *pia,
 			have_values = true;
 			appendPQExpBuffer(buf, "%s\n(%d, ", comma, pattern_id);
 			appendStringLiteralConn(buf, info->db_regex, conn);
-			appendPQExpBufferStr(buf, ")");
+			appendPQExpBufferChar(buf, ')');
 			comma = ",";
 		}
 	}
@@ -1765,7 +1765,7 @@ append_rel_pattern_raw_cte(PQExpBuffer buf, const PatternInfoArray *pia,
 			appendPQExpBufferStr(buf, ", true::BOOLEAN");
 		else
 			appendPQExpBufferStr(buf, ", false::BOOLEAN");
-		appendPQExpBufferStr(buf, ")");
+		appendPQExpBufferChar(buf, ')');
 		comma = ",";
 	}
 
@@ -1972,15 +1972,15 @@ compile_relation_list_one_db(PGconn *conn, SimplePtrList *relations,
 		 * selected above, filtering by exclusion patterns (if any) that match
 		 * btree index names.
 		 */
-		appendPQExpBuffer(&sql,
-						  ", index (oid, nspname, relname, relpages) AS ("
-						  "\nSELECT c.oid, r.nspname, c.relname, c.relpages "
-						  "FROM relation r"
-						  "\nINNER JOIN pg_catalog.pg_index i "
-						  "ON r.oid = i.indrelid "
-						  "INNER JOIN pg_catalog.pg_class c "
-						  "ON i.indexrelid = c.oid "
-						  "AND c.relpersistence != 't'");
+		appendPQExpBufferStr(&sql,
+							 ", index (oid, nspname, relname, relpages) AS ("
+							 "\nSELECT c.oid, r.nspname, c.relname, c.relpages "
+							 "FROM relation r"
+							 "\nINNER JOIN pg_catalog.pg_index i "
+							 "ON r.oid = i.indrelid "
+							 "INNER JOIN pg_catalog.pg_class c "
+							 "ON i.indexrelid = c.oid "
+							 "AND c.relpersistence != 't'");
 		if (opts.excludeidx || opts.excludensp)
 			appendPQExpBufferStr(&sql,
 								 "\nINNER JOIN pg_catalog.pg_namespace n "
@@ -2011,15 +2011,15 @@ compile_relation_list_one_db(PGconn *conn, SimplePtrList *relations,
 		 * primary heap tables selected above, filtering by exclusion patterns
 		 * (if any) that match the toast index names.
 		 */
-		appendPQExpBuffer(&sql,
-						  ", toast_index (oid, nspname, relname, relpages) AS ("
-						  "\nSELECT c.oid, 'pg_toast', c.relname, c.relpages "
-						  "FROM toast t "
-						  "INNER JOIN pg_catalog.pg_index i "
-						  "ON t.oid = i.indrelid"
-						  "\nINNER JOIN pg_catalog.pg_class c "
-						  "ON i.indexrelid = c.oid "
-						  "AND c.relpersistence != 't'");
+		appendPQExpBufferStr(&sql,
+							 ", toast_index (oid, nspname, relname, relpages) AS ("
+							 "\nSELECT c.oid, 'pg_toast', c.relname, c.relpages "
+							 "FROM toast t "
+							 "INNER JOIN pg_catalog.pg_index i "
+							 "ON t.oid = i.indrelid"
+							 "\nINNER JOIN pg_catalog.pg_class c "
+							 "ON i.indexrelid = c.oid "
+							 "AND c.relpersistence != 't'");
 		if (opts.excludeidx)
 			appendPQExpBufferStr(&sql,
 								 "\nLEFT OUTER JOIN exclude_pat ep "
@@ -2044,9 +2044,9 @@ compile_relation_list_one_db(PGconn *conn, SimplePtrList *relations,
 	 * matched in their own right, so we rely on UNION to deduplicate the
 	 * list.
 	 */
-	appendPQExpBuffer(&sql,
-					  "\nSELECT pattern_id, is_heap, is_btree, oid, nspname, relname, relpages "
-					  "FROM (");
+	appendPQExpBufferStr(&sql,
+						 "\nSELECT pattern_id, is_heap, is_btree, oid, nspname, relname, relpages "
+						 "FROM (");
 	appendPQExpBufferStr(&sql,
 	/* Inclusion patterns that failed to match */
 						 "\nSELECT pattern_id, is_heap, is_btree, "