Re: Remove unnecessary static type qualifiers

From: Junwang Zhao <zhjwpku(at)gmail(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: Peter Eisentraut <peter(at)eisentraut(dot)org>, Japin Li <japinli(at)hotmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Remove unnecessary static type qualifiers
Date: 2025-04-09 02:34:45
Message-ID: CAEG8a3KQ+sXXTDQhHPzX1B8MM0jDPoEQyedOPzrC9JnCd0HWJw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Apr 9, 2025 at 5:22 AM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>
> On Wed, 9 Apr 2025 at 03:46, Peter Eisentraut <peter(at)eisentraut(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jacob Champion 2025-04-09 02:57:47 Re: [PoC] Federated Authn/z with OAUTHBEARER
Previous Message Euler Taveira 2025-04-09 02:21:31 Re: Large expressions in indexes can't be stored (non-TOASTable)