Postgres Pro
Downloads
Home   >   mailing lists

Thread: remove unnecessary explicit type conversion (to char) for appendStringInfoChar function calls

remove unnecessary explicit type conversion (to char) for appendStringInfoChar function calls

From
Mahendra Singh Thalor
Date:
Hi,
In the current master code, 3 places we are using appendStringInfoChar
call with explicit type conversion into char. This is not needed as
all other places, we are using direct character to append.

--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -302,7 +302,7 @@ InteractiveBackend(StringInfo inBuf)
         */

        /* Add '\0' to make it look the same as message case. */
-       appendStringInfoChar(inBuf, (char) '\0');
+       appendStringInfoChar(inBuf, '\0');

Here, I am attaching a small patch to fix these 3 type conversions on head.

-- 
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com
Attachment

Re: remove unnecessary explicit type conversion (to char) for appendStringInfoChar function calls

From
Andrew Dunstan
Date:
On 2025-04-11 Fr 1:36 PM, Mahendra Singh Thalor wrote:
> Hi,
> In the current master code, 3 places we are using appendStringInfoChar
> call with explicit type conversion into char. This is not needed as
> all other places, we are using direct character to append.
>
> --- a/src/backend/tcop/postgres.c
> +++ b/src/backend/tcop/postgres.c
> @@ -302,7 +302,7 @@ InteractiveBackend(StringInfo inBuf)
>           */
>
>          /* Add '\0' to make it look the same as message case. */
> -       appendStringInfoChar(inBuf, (char) '\0');
> +       appendStringInfoChar(inBuf, '\0');
>
> Here, I am attaching a small patch to fix these 3 type conversions on head.
>


Seems odd that this one is necessary at all. Doesn't a StringInfo always 
have a trailing null byte?


cheers


andrew



--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Re: remove unnecessary explicit type conversion (to char) for appendStringInfoChar function calls

From
Mahendra Singh Thalor
Date:
On Sat, 12 Apr 2025 at 23:56, Andrew Dunstan <andrew@dunslane.net> wrote:
>
>
> On 2025-04-11 Fr 1:36 PM, Mahendra Singh Thalor wrote:
> > Hi,
> > In the current master code, 3 places we are using appendStringInfoChar
> > call with explicit type conversion into char. This is not needed as
> > all other places, we are using direct character to append.
> >
> > --- a/src/backend/tcop/postgres.c
> > +++ b/src/backend/tcop/postgres.c
> > @@ -302,7 +302,7 @@ InteractiveBackend(StringInfo inBuf)
> >           */
> >
> >          /* Add '\0' to make it look the same as message case. */
> > -       appendStringInfoChar(inBuf, (char) '\0');
> > +       appendStringInfoChar(inBuf, '\0');
> >
> > Here, I am attaching a small patch to fix these 3 type conversions on head.
> >
>
>
> Seems odd that this one is necessary at all. Doesn't a StringInfo always
> have a trailing null byte?
>
>
> cheers
>
>
> andrew
>
>
>
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com
>

Yes, we already have a NULL byte in the message but as per current code, we can't remove this call because msg->cursor is coded as per extra NULL byte.

diff --git a/src/backend/libpq/pqformat.c b/src/backend/libpq/pqformat.c
index 1cc126772f7..979ba9e48cf 100644
--- a/src/backend/libpq/pqformat.c
+++ b/src/backend/libpq/pqformat.c
@@ -589,11 +589,11 @@ pq_getmsgstring(StringInfo msg)
         * message.
         */
        slen = strlen(str);
-       if (msg->cursor + slen >= msg->len)
+       if (msg->cursor + slen > msg->len)
                ereport(ERROR,
                                (errcode(ERRCODE_PROTOCOL_VIOLATION),
                                 errmsg("invalid string in message")));
-       msg->cursor += slen + 1;
+       msg->cursor += slen;
 
        return pg_client_to_server(str, slen);
 }
@@ -618,11 +618,11 @@ pq_getmsgrawstring(StringInfo msg)
         * message.
         */
        slen = strlen(str);
-       if (msg->cursor + slen >= msg->len)
+       if (msg->cursor + slen > msg->len)
                ereport(ERROR,
                                (errcode(ERRCODE_PROTOCOL_VIOLATION),
                                 errmsg("invalid string in message")));
-       msg->cursor += slen + 1;
+       msg->cursor += slen;
 
        return str;
 }

We need many more changes if we want to remove NULL byte call.

Above changes will fix errors of initdb but other tests are failing as code expects this extra null byte in string.

--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com

Re: remove unnecessary explicit type conversion (to char) for appendStringInfoChar function calls

From
Álvaro Herrera
Date:
On 2025-Apr-12, Andrew Dunstan wrote:

> Seems odd that this one is necessary at all. Doesn't a StringInfo always
> have a trailing null byte?

AFAICT what this is doing that's significant, is increment StringInfo->len.
Before doing it, the NUL byte is not part of the message; afterwards it
is.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"The important things in the world are problems with society that we don't
understand at all. The machines will become more complicated but they won't
be more complicated than the societies that run them."    (Freeman Dyson)

Re: remove unnecessary explicit type conversion (to char) for appendStringInfoChar function calls

From
Andrew Dunstan
Date:
On 2025-04-13 Su 8:12 AM, Álvaro Herrera wrote:
> On 2025-Apr-12, Andrew Dunstan wrote:
>
>> Seems odd that this one is necessary at all. Doesn't a StringInfo always
>> have a trailing null byte?
> AFAICT what this is doing that's significant, is increment StringInfo->len.
> Before doing it, the NUL byte is not part of the message; afterwards it
> is.
>


That make sense, but then it would be nice to have the accompanying 
comment a bit clearer.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com