Use PointerGetDatum(cstring_to_text_with_len()) instead of CStringGetTextDatum() to avoid duplicate strlen
Hi
I found some code like the following:
StringInfoData s;
...
values[6] = CStringGetTextDatum(s.data);
The length of string can be found in ' StringInfoData.len',
but the macro CStringGetTextDatum will use strlen to count the length again.
I think we can use PointerGetDatum(cstring_to_text_with_len(s.data, s.len)) to improve.
#define CStringGetTextDatum(s) PointerGetDatum(cstring_to_text(s))
text *
cstring_to_text(const char *s)
{
return cstring_to_text_with_len(s, strlen(s));
}
There may have more places that can get the length of string in advance,
But that may need new variable to store it ,So I just find all StringInfoData cases.
Best regards,
houzj
Attachments:
0001-prevent-duplicate-strlen.patchapplication/octet-stream; name=0001-prevent-duplicate-strlen.patchDownload
From 9ebb8793e7546c6edf3aaea6acb6999f8ca8938a Mon Sep 17 00:00:00 2001
From: root <root@localhost.localdomain>
Date: Mon, 19 Oct 2020 02:11:41 -0400
Subject: [PATCH] Use PointerGetDatum(cstring_to_text_with_len(str, len))
instead of CStringGetTextDatum(str) to avoid duplicate strlen()
---
contrib/pageinspect/brinfuncs.c | 2 +-
src/backend/access/transam/xlogfuncs.c | 4 ++--
src/backend/commands/extension.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c
index fb32d74..87df4c8 100644
--- a/contrib/pageinspect/brinfuncs.c
+++ b/contrib/pageinspect/brinfuncs.c
@@ -282,7 +282,7 @@ brin_page_items(PG_FUNCTION_ARGS)
}
appendStringInfoChar(&s, '}');
- values[6] = CStringGetTextDatum(s.data);
+ values[6] = PointerGetDatum(cstring_to_text_with_len(s.data, s.len));
pfree(s.data);
}
else
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 290658b..50cd532 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -236,8 +236,8 @@ pg_stop_backup_v2(PG_FUNCTION_ARGS)
*/
stoppoint = do_pg_stop_backup(label_file->data, waitforarchive, NULL);
- values[1] = CStringGetTextDatum(label_file->data);
- values[2] = CStringGetTextDatum(tblspc_map_file->data);
+ values[1] = PointerGetDatum(cstring_to_text_with_len(label_file->data, label_file->len));
+ values[2] = PointerGetDatum(cstring_to_text_with_len(tblspc_map_file->data, tblspc_map_file->len));
/* Free structures allocated in TopMemoryContext */
pfree(label_file->data);
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index b5630b4..2fea258 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -2406,7 +2406,7 @@ pg_extension_update_paths(PG_FUNCTION_ARGS)
appendStringInfoString(&pathbuf, "--");
appendStringInfoString(&pathbuf, versionName);
}
- values[2] = CStringGetTextDatum(pathbuf.data);
+ values[2] = PointerGetDatum(cstring_to_text_with_len(pathbuf.data, pathbuf.len));
pfree(pathbuf.data);
}
--
1.8.3.1
On 19/10/2020 09:32, Hou, Zhijie wrote:
Hi
I found some code like the following:
StringInfoData s;
...
values[6] = CStringGetTextDatum(s.data);The length of string can be found in ' StringInfoData.len',
but the macro CStringGetTextDatum will use strlen to count the length again.
I think we can use PointerGetDatum(cstring_to_text_with_len(s.data, s.len)) to improve.#define CStringGetTextDatum(s) PointerGetDatum(cstring_to_text(s))
text *
cstring_to_text(const char *s)
{
return cstring_to_text_with_len(s, strlen(s));
}There may have more places that can get the length of string in advance,
But that may need new variable to store it ,So I just find all StringInfoData cases.
None of these calls are performance-critical, so it hardly matters. I
would rather keep them short and simple.
It might make sense to create a new macro or function for this, though.
Something like:
static inline text *
StringInfoGetTextDatum(StringInfo s)
{
return cstring_to_text_with_len(s->data, s->len);
}
That would perhaps make existing code a bit shorter and nicer to read.
- Heikki