public inbox for [email protected]
help / color / mirror / Atom feed[PATCH] ecpg: use memcpy in a few length-based copies
2+ messages / 2 participants
[nested] [flat]
* [PATCH] ecpg: use memcpy in a few length-based copies
@ 2026-07-08 19:24 Haibo Yan <[email protected]>
2026-07-09 07:04 ` Re: [PATCH] ecpg: use memcpy in a few length-based copies Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 2+ messages in thread
From: Haibo Yan @ 2026-07-08 19:24 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
Hi hackers,
I noticed a few places in ecpg that use strncpy() even though the code already
knows how many bytes to copy.
For example, some paths copy N bytes into a temporary buffer and then add the
terminating NUL explicitly. There is also one small substring copy in
pgtypeslib/datetime.c. memcpy() seems a better fit for those cases.
I did not try to replace all strncpy() calls in ecpg. Some of them still need
the usual strncpy() behavior when the source string may be shorter than the
destination.
The patch also adds a small regression test for fixed-size char[] and VARCHAR
output around the exact-fit/truncation boundary.
Thanks,
Haibo
Attachments:
[application/octet-stream] 0001-ecpg-use-memcpy-for-selected-length-based-buffer-cop.patch (14.4K, ../../CABXr29HdcPfU+wC7Va9Z5WXh+Kai_63J4J-e3d-_w=EuBNFKLw@mail.gmail.com/2-0001-ecpg-use-memcpy-for-selected-length-based-buffer-cop.patch)
download | inline diff:
From 9b5e6ad6f5eb62693f64dc8ecee4e4dec1180150 Mon Sep 17 00:00:00 2001
From: Haibo Yan <[email protected]>
Date: Wed, 8 Jul 2026 10:03:13 -0700
Subject: [PATCH] ecpg: use memcpy for selected length-based buffer copies
Several ECPG code paths used strncpy() even though the surrounding code
does not rely on strncpy()'s zero-padding behavior. In the changed sites,
the copy length is already bounded explicitly, and the destination is either
a temporary buffer that is explicitly NUL-terminated immediately afterwards,
a counted ECPG host-variable buffer, or an internal substring-copy target.
Using strncpy() in such cases is misleading because it has fixed-width string
semantics: it stops at the first NUL byte in the source and pads the
remaining destination bytes with NULs. For the changed call sites, that
padding is not needed and the copied range is already known to be in bounds.
Use memcpy() to make the length-based copy explicit while preserving the
existing explicit NUL termination and truncation behavior.
Some other strncpy() calls in ECPG are intentionally left unchanged because
they may copy from shorter source strings into fixed-size destinations, where
a naive memcpy() would read past the source NUL or would fail to preserve
padding behavior.
A regression test (sql/chartrunc) is added to lock down the fixed-size char[]
and VARCHAR output behavior at the buffer boundary (shorter than buffer,
exact fit, and truncation), including the truncation indicator and
sqlca.sqlwarn handling.
This is a cleanup intended to clarify the buffer-copy semantics; it should
not change user-visible ECPG behavior.
---
src/interfaces/ecpg/ecpglib/data.c | 6 +-
src/interfaces/ecpg/ecpglib/execute.c | 6 +-
src/interfaces/ecpg/pgtypeslib/datetime.c | 4 +-
src/interfaces/ecpg/test/ecpg_schedule | 1 +
.../ecpg/test/expected/sql-chartrunc.c | 220 ++++++++++++++++++
.../ecpg/test/expected/sql-chartrunc.stderr | 0
.../ecpg/test/expected/sql-chartrunc.stdout | 3 +
src/interfaces/ecpg/test/sql/Makefile | 1 +
src/interfaces/ecpg/test/sql/chartrunc.pgc | 53 +++++
src/interfaces/ecpg/test/sql/meson.build | 1 +
10 files changed, 287 insertions(+), 8 deletions(-)
create mode 100644 src/interfaces/ecpg/test/expected/sql-chartrunc.c
create mode 100644 src/interfaces/ecpg/test/expected/sql-chartrunc.stderr
create mode 100644 src/interfaces/ecpg/test/expected/sql-chartrunc.stdout
create mode 100644 src/interfaces/ecpg/test/sql/chartrunc.pgc
diff --git a/src/interfaces/ecpg/ecpglib/data.c b/src/interfaces/ecpg/ecpglib/data.c
index d5d40f7b654..b7263735ca9 100644
--- a/src/interfaces/ecpg/ecpglib/data.c
+++ b/src/interfaces/ecpg/ecpglib/data.c
@@ -611,7 +611,7 @@ ecpg_get_data(const PGresult *results, int act_tuple, int act_field, int lineno,
}
else
{
- strncpy(str, pval, size + 1);
+ memcpy(str, pval, size + 1);
}
/* do the rtrim() */
if (type == ECPGt_string)
@@ -636,7 +636,7 @@ ecpg_get_data(const PGresult *results, int act_tuple, int act_field, int lineno,
if (varcharsize == 0)
charsize = size + 1;
- strncpy(str, pval, charsize);
+ memcpy(str, pval, charsize);
/* compatibility mode, null terminate char array */
if (ORACLE_MODE(compat) && (charsize - 1) < size)
@@ -683,7 +683,7 @@ ecpg_get_data(const PGresult *results, int act_tuple, int act_field, int lineno,
variable->len = size;
if (varcharsize == 0)
- strncpy(variable->arr, pval, variable->len);
+ memcpy(variable->arr, pval, variable->len);
else
{
strncpy(variable->arr, pval, varcharsize);
diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c
index 02380dafd9d..9c05a465b8a 100644
--- a/src/interfaces/ecpg/ecpglib/execute.c
+++ b/src/interfaces/ecpg/ecpglib/execute.c
@@ -793,7 +793,7 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari
if (!(newcopy = ecpg_alloc(slen + 1, lineno)))
return false;
- strncpy(newcopy, (char *) var->value, slen);
+ memcpy(newcopy, (char *) var->value, slen);
newcopy[slen] = '\0';
mallocedval = quote_postgres(newcopy, quote, lineno);
@@ -814,7 +814,7 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari
if (!(mallocedval = ecpg_alloc(slen + 1, lineno)))
return false;
- strncpy(mallocedval, (char *) var->value, slen);
+ memcpy(mallocedval, (char *) var->value, slen);
mallocedval[slen] = '\0';
*tobeinserted_p = mallocedval;
@@ -842,7 +842,7 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari
if (!(newcopy = ecpg_alloc(variable->len + 1, lineno)))
return false;
- strncpy(newcopy, variable->arr, variable->len);
+ memcpy(newcopy, variable->arr, variable->len);
newcopy[variable->len] = '\0';
mallocedval = quote_postgres(newcopy, quote, lineno);
diff --git a/src/interfaces/ecpg/pgtypeslib/datetime.c b/src/interfaces/ecpg/pgtypeslib/datetime.c
index f43343b4594..82356adc538 100644
--- a/src/interfaces/ecpg/pgtypeslib/datetime.c
+++ b/src/interfaces/ecpg/pgtypeslib/datetime.c
@@ -489,8 +489,8 @@ PGTYPESdate_defmt_asc(date * d, const char *fmt, const char *str)
if (i == 2)
start_pos += frag_length[1];
- strncpy(str_copy + target_pos, str + start_pos,
- frag_length[i]);
+ memcpy(str_copy + target_pos, str + start_pos,
+ frag_length[i]);
target_pos += frag_length[i];
if (i != 2)
{
diff --git a/src/interfaces/ecpg/test/ecpg_schedule b/src/interfaces/ecpg/test/ecpg_schedule
index d1f5d9452b7..1bb2e9f1f3c 100644
--- a/src/interfaces/ecpg/test/ecpg_schedule
+++ b/src/interfaces/ecpg/test/ecpg_schedule
@@ -35,6 +35,7 @@ test: preproc/whenever_do_continue
test: sql/array
test: sql/binary
test: sql/bytea
+test: sql/chartrunc
test: sql/code100
test: sql/copystdout
test: sql/createtableas
diff --git a/src/interfaces/ecpg/test/expected/sql-chartrunc.c b/src/interfaces/ecpg/test/expected/sql-chartrunc.c
new file mode 100644
index 00000000000..102a502a2fe
--- /dev/null
+++ b/src/interfaces/ecpg/test/expected/sql-chartrunc.c
@@ -0,0 +1,220 @@
+/* Processed by ecpg (regression mode) */
+/* These include files are added by the preprocessor */
+#include <ecpglib.h>
+#include <ecpgerrno.h>
+#include <sqlca.h>
+/* End of automatic include section */
+#define ECPGdebug(X,Y) ECPGdebug((X)+100,(Y))
+
+#line 1 "chartrunc.pgc"
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+
+#line 1 "sqlca.h"
+#ifndef POSTGRES_SQLCA_H
+#define POSTGRES_SQLCA_H
+
+#ifndef PGDLLIMPORT
+#if defined(WIN32) || defined(__CYGWIN__)
+#define PGDLLIMPORT __declspec (dllimport)
+#else
+#define PGDLLIMPORT
+#endif /* __CYGWIN__ */
+#endif /* PGDLLIMPORT */
+
+#define SQLERRMC_LEN 150
+
+#ifdef __cplusplus
+extern "C"
+{
+#endif
+
+struct sqlca_t
+{
+ char sqlcaid[8];
+ long sqlabc;
+ long sqlcode;
+ struct
+ {
+ int sqlerrml;
+ char sqlerrmc[SQLERRMC_LEN];
+ } sqlerrm;
+ char sqlerrp[8];
+ long sqlerrd[6];
+ /* Element 0: empty */
+ /* 1: OID of processed tuple if applicable */
+ /* 2: number of rows processed */
+ /* after an INSERT, UPDATE or */
+ /* DELETE statement */
+ /* 3: empty */
+ /* 4: empty */
+ /* 5: empty */
+ char sqlwarn[8];
+ /* Element 0: set to 'W' if at least one other is 'W' */
+ /* 1: if 'W' at least one character string */
+ /* value was truncated when it was */
+ /* stored into a host variable. */
+
+ /*
+ * 2: if 'W' a (hopefully) non-fatal notice occurred
+ */ /* 3: empty */
+ /* 4: empty */
+ /* 5: empty */
+ /* 6: empty */
+ /* 7: empty */
+
+ char sqlstate[5];
+};
+
+struct sqlca_t *ECPGget_sqlca(void);
+
+#ifndef POSTGRES_ECPG_INTERNAL
+#define sqlca (*ECPGget_sqlca())
+#endif
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
+
+#line 5 "chartrunc.pgc"
+
+
+#line 1 "regression.h"
+
+
+
+
+
+
+#line 6 "chartrunc.pgc"
+
+
+/*
+ * Test fixed-size char[] and VARCHAR output around the buffer boundary in
+ * regular mode. char[] is printed with a precision because exact-fit and
+ * truncated values are not NUL-terminated.
+ */
+int main(void)
+{
+ /* exec sql begin declare section */
+
+
+
+
+
+#line 16 "chartrunc.pgc"
+ char c1 [ 4 ] ;
+
+#line 17 "chartrunc.pgc"
+ struct varchar_1 { int len; char arr[ 4 ]; } v1 ;
+
+#line 18 "chartrunc.pgc"
+ short c1_ind ;
+
+#line 19 "chartrunc.pgc"
+ short v1_ind ;
+/* exec sql end declare section */
+#line 20 "chartrunc.pgc"
+
+
+ { ECPGconnect(__LINE__, 0, "ecpg1_regression" , NULL, NULL , NULL, 0); }
+#line 22 "chartrunc.pgc"
+
+ /* exec sql whenever sqlerror stop ; */
+#line 23 "chartrunc.pgc"
+
+
+ { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "create table ecpg_chartrunc ( id int , val varchar ( 10 ) )", ECPGt_EOIT, ECPGt_EORT);
+#line 25 "chartrunc.pgc"
+
+if (sqlca.sqlcode < 0) exit (1);}
+#line 25 "chartrunc.pgc"
+
+ { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "insert into ecpg_chartrunc values ( 1 , 'abc' )", ECPGt_EOIT, ECPGt_EORT);
+#line 26 "chartrunc.pgc"
+
+if (sqlca.sqlcode < 0) exit (1);}
+#line 26 "chartrunc.pgc"
+
+ { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "insert into ecpg_chartrunc values ( 2 , 'abcd' )", ECPGt_EOIT, ECPGt_EORT);
+#line 27 "chartrunc.pgc"
+
+if (sqlca.sqlcode < 0) exit (1);}
+#line 27 "chartrunc.pgc"
+
+ { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "insert into ecpg_chartrunc values ( 3 , 'abcde' )", ECPGt_EOIT, ECPGt_EORT);
+#line 28 "chartrunc.pgc"
+
+if (sqlca.sqlcode < 0) exit (1);}
+#line 28 "chartrunc.pgc"
+
+
+ /* declare cur cursor for select val , val from ecpg_chartrunc order by id */
+#line 30 "chartrunc.pgc"
+
+ { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "declare cur cursor for select val , val from ecpg_chartrunc order by id", ECPGt_EOIT, ECPGt_EORT);
+#line 31 "chartrunc.pgc"
+
+if (sqlca.sqlcode < 0) exit (1);}
+#line 31 "chartrunc.pgc"
+
+
+ /* exec sql whenever not found break ; */
+#line 33 "chartrunc.pgc"
+
+ for (;;)
+ {
+ memset(c1, 0, sizeof(c1));
+ memset(v1.arr, 0, sizeof(v1.arr));
+ v1.len = 0;
+ c1_ind = v1_ind = 0;
+ { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "fetch cur", ECPGt_EOIT,
+ ECPGt_char,(c1),(long)4,(long)1,(4)*sizeof(char),
+ ECPGt_short,&(c1_ind),(long)1,(long)1,sizeof(short),
+ ECPGt_varchar,&(v1),(long)4,(long)1,sizeof(struct varchar_1),
+ ECPGt_short,&(v1_ind),(long)1,(long)1,sizeof(short), ECPGt_EORT);
+#line 40 "chartrunc.pgc"
+
+if (sqlca.sqlcode == ECPG_NOT_FOUND) break;
+#line 40 "chartrunc.pgc"
+
+if (sqlca.sqlcode < 0) exit (1);}
+#line 40 "chartrunc.pgc"
+
+ printf("c1=\"%.4s\" c1_ind=%d warn=%c | v1.len=%d v1.arr=\"%.*s\" v1_ind=%d\n",
+ c1, c1_ind, (sqlca.sqlwarn[1] == 'W') ? 'W' : '-',
+ v1.len, v1.len, v1.arr, v1_ind);
+ }
+
+ { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "close cur", ECPGt_EOIT, ECPGt_EORT);
+#line 46 "chartrunc.pgc"
+
+if (sqlca.sqlcode < 0) exit (1);}
+#line 46 "chartrunc.pgc"
+
+ { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "drop table ecpg_chartrunc", ECPGt_EOIT, ECPGt_EORT);
+#line 47 "chartrunc.pgc"
+
+if (sqlca.sqlcode < 0) exit (1);}
+#line 47 "chartrunc.pgc"
+
+ { ECPGtrans(__LINE__, NULL, "commit work");
+#line 48 "chartrunc.pgc"
+
+if (sqlca.sqlcode < 0) exit (1);}
+#line 48 "chartrunc.pgc"
+
+
+ { ECPGdisconnect(__LINE__, "ALL");
+#line 50 "chartrunc.pgc"
+
+if (sqlca.sqlcode < 0) exit (1);}
+#line 50 "chartrunc.pgc"
+
+
+ return 0;
+}
diff --git a/src/interfaces/ecpg/test/expected/sql-chartrunc.stderr b/src/interfaces/ecpg/test/expected/sql-chartrunc.stderr
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/src/interfaces/ecpg/test/expected/sql-chartrunc.stdout b/src/interfaces/ecpg/test/expected/sql-chartrunc.stdout
new file mode 100644
index 00000000000..37ae859a2cd
--- /dev/null
+++ b/src/interfaces/ecpg/test/expected/sql-chartrunc.stdout
@@ -0,0 +1,3 @@
+c1="abc" c1_ind=0 warn=- | v1.len=3 v1.arr="abc" v1_ind=0
+c1="abcd" c1_ind=0 warn=- | v1.len=4 v1.arr="abcd" v1_ind=0
+c1="abcd" c1_ind=5 warn=W | v1.len=4 v1.arr="abcd" v1_ind=5
diff --git a/src/interfaces/ecpg/test/sql/Makefile b/src/interfaces/ecpg/test/sql/Makefile
index 5296848cda9..b6daec7e033 100644
--- a/src/interfaces/ecpg/test/sql/Makefile
+++ b/src/interfaces/ecpg/test/sql/Makefile
@@ -5,6 +5,7 @@ include $(top_srcdir)/$(subdir)/../Makefile.regress
TESTS = array array.c \
binary binary.c \
+ chartrunc chartrunc.c \
code100 code100.c \
copystdout copystdout.c \
createtableas createtableas.c \
diff --git a/src/interfaces/ecpg/test/sql/chartrunc.pgc b/src/interfaces/ecpg/test/sql/chartrunc.pgc
new file mode 100644
index 00000000000..98de4ca7caf
--- /dev/null
+++ b/src/interfaces/ecpg/test/sql/chartrunc.pgc
@@ -0,0 +1,53 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+exec sql include sqlca;
+exec sql include ../regression;
+
+/*
+ * Test fixed-size char[] and VARCHAR output around the buffer boundary in
+ * regular mode. char[] is printed with a precision because exact-fit and
+ * truncated values are not NUL-terminated.
+ */
+int main(void)
+{
+ exec sql begin declare section;
+ char c1[4];
+ VARCHAR v1[4];
+ short c1_ind;
+ short v1_ind;
+ exec sql end declare section;
+
+ exec sql connect to REGRESSDB1;
+ exec sql whenever sqlerror stop;
+
+ exec sql create table ecpg_chartrunc (id int, val varchar(10));
+ exec sql insert into ecpg_chartrunc values (1, 'abc');
+ exec sql insert into ecpg_chartrunc values (2, 'abcd');
+ exec sql insert into ecpg_chartrunc values (3, 'abcde');
+
+ exec sql declare cur cursor for select val, val from ecpg_chartrunc order by id;
+ exec sql open cur;
+
+ exec sql whenever not found do break;
+ for (;;)
+ {
+ memset(c1, 0, sizeof(c1));
+ memset(v1.arr, 0, sizeof(v1.arr));
+ v1.len = 0;
+ c1_ind = v1_ind = 0;
+ exec sql fetch cur into :c1 :c1_ind, :v1 :v1_ind;
+ printf("c1=\"%.4s\" c1_ind=%d warn=%c | v1.len=%d v1.arr=\"%.*s\" v1_ind=%d\n",
+ c1, c1_ind, (sqlca.sqlwarn[1] == 'W') ? 'W' : '-',
+ v1.len, v1.len, v1.arr, v1_ind);
+ }
+
+ exec sql close cur;
+ exec sql drop table ecpg_chartrunc;
+ exec sql commit work;
+
+ exec sql disconnect all;
+
+ return 0;
+}
diff --git a/src/interfaces/ecpg/test/sql/meson.build b/src/interfaces/ecpg/test/sql/meson.build
index 00d7debe80f..adb7f48ed68 100644
--- a/src/interfaces/ecpg/test/sql/meson.build
+++ b/src/interfaces/ecpg/test/sql/meson.build
@@ -4,6 +4,7 @@ pgc_files = [
'array',
'binary',
'bytea',
+ 'chartrunc',
'code100',
'copystdout',
'createtableas',
--
2.54.0
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: [PATCH] ecpg: use memcpy in a few length-based copies
2026-07-08 19:24 [PATCH] ecpg: use memcpy in a few length-based copies Haibo Yan <[email protected]>
@ 2026-07-09 07:04 ` Peter Eisentraut <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Peter Eisentraut @ 2026-07-09 07:04 UTC (permalink / raw)
To: Haibo Yan <[email protected]>; PostgreSQL Hackers <[email protected]>
On 08.07.26 21:24, Haibo Yan wrote:
> I noticed a few places in ecpg that use strncpy() even though the code already
> knows how many bytes to copy.
>
> For example, some paths copy N bytes into a temporary buffer and then add the
> terminating NUL explicitly. There is also one small substring copy in
> pgtypeslib/datetime.c. memcpy() seems a better fit for those cases.
Why is it better? At least strncpy() enforces that the target is a char
array, which memcpy() doesn't.
At a quick glance, strlcpy() might be more suitable in some of the cases
you found.
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-07-09 07:04 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-07-08 19:24 [PATCH] ecpg: use memcpy in a few length-based copies Haibo Yan <[email protected]>
2026-07-09 07:04 ` Peter Eisentraut <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox