On Wed, Apr 9, 2025 at 5:22 AM David Rowley <dgrowle...@gmail.com> wrote: > > On Wed, 9 Apr 2025 at 03:46, Peter Eisentraut <pe...@eisentraut.org> wrote: > > To avoid creating an array on the stack, you could maybe write it with a > > pointer instead, like: > > > > const char * const query = "..."; > > > > I haven't tested whether that results in different or better compiled > > code. The original code is probably good enough. > > I expect it's been done the way it has to make the overflow detection > code work. The problem with having a pointer to a string constant is > that sizeof() will return the size of the pointer and not the space > needed to store the string. > > We can get rid of the variable and make the overflow work by checking > the return length of snprintf. I think that's all C99 spec now... > > len = snprintf(qbuf, "set client_encoding to '%s'", encoding); > /* check query buffer overflow */ > if (len >= sizeof(qbuf)) > return -1; >
Agree, this is a better coding style. Please see if the following code makes sense to you. diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 0258d9ace3c..247d079faa6 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -7717,9 +7717,9 @@ int PQsetClientEncoding(PGconn *conn, const char *encoding) { char qbuf[128]; - static const char query[] = "set client_encoding to '%s'"; PGresult *res; int status; + int len; if (!conn || conn->status != CONNECTION_OK) return -1; @@ -7731,12 +7731,11 @@ PQsetClientEncoding(PGconn *conn, const char *encoding) if (strcmp(encoding, "auto") == 0) encoding = pg_encoding_to_char(pg_get_encoding_from_locale(NULL, true)); - /* check query buffer overflow */ - if (sizeof(qbuf) < (sizeof(query) + strlen(encoding))) + len = snprintf(qbuf, sizeof(qbuf), "set client_encoding to '%s'", encoding); + if (len >= sizeof(qbuf)) return -1; /* ok, now send a query */ - sprintf(qbuf, query, encoding); res = PQexec(conn, qbuf); if (res == NULL) > David -- Regards Junwang Zhao