Обсуждение: [PATCH]Remove the redundant assignment

Hi,Hackers,

I found that in the InjectionPointAttach function within the src/backend/utils/misc/injection_point.c file, the
variableis manually assigned a '\0' at the end, even though strlcpy already guarantees that the destination buffer will
benull-terminated and will not overflow. 

The code modification is as follows:

```
/* Save the entry */
strlcpy(entry->name, name, sizeof(entry->name));
entry->name[INJ_NAME_MAXLEN - 1] = '\0';  <== Delete this line
strlcpy(entry->library, library, sizeof(entry->library));
entry->library[INJ_LIB_MAXLEN - 1] = '\0';  <== Delete this line
strlcpy(entry->function, function, sizeof(entry->function));
entry->function[INJ_FUNC_MAXLEN - 1] = '\0';  <== Delete this line
```

And in the injection_point_cache_add function within the same file, strlcpy(entry->name, name, sizeof(entry->name));
doesnot perform redundant assignment. 

I have tested the change, and "make check" passed.



--------------

Best regards,


Feilong Meng

> On Dec 16, 2025, at 14:18, Feilong Meng <feelingmeng@foxmail.com> wrote:
>
> Hi,Hackers,
>
> I found that in the InjectionPointAttach function within the src/backend/utils/misc/injection_point.c file, the
variableis manually assigned a '\0' at the end, even though strlcpy already guarantees that the destination buffer will
benull-terminated and will not overflow. 
>
> The code modification is as follows:
>
> ```
> /* Save the entry */
> strlcpy(entry->name, name, sizeof(entry->name));
> entry->name[INJ_NAME_MAXLEN - 1] = '\0';  <== Delete this line
> strlcpy(entry->library, library, sizeof(entry->library));
> entry->library[INJ_LIB_MAXLEN - 1] = '\0';  <== Delete this line
> strlcpy(entry->function, function, sizeof(entry->function));
> entry->function[INJ_FUNC_MAXLEN - 1] = '\0';  <== Delete this line
> ```
>
> And in the injection_point_cache_add function within the same file, strlcpy(entry->name, name, sizeof(entry->name));
doesnot perform redundant assignment. 
>
> I have tested the change, and "make check" passed.
>

Indeed, explicitly adding a trailing '\0' is redundant here, since strlcpy() already guarantees NULL termination.

I also noticed that in the same file, another code path does not perform this extra assignment:

```
Assert(!found);
strlcpy(entry->name, name, sizeof(entry->name));
entry->slot_idx = slot_idx;
entry->generation = generation;
entry->callback = callback;
memcpy(entry->private_data, private_data, INJ_PRIVATE_MAXLEN);
```

Given that, I agree we should remove the redundant assignments to keep the code clearer and consistent.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Tue, Dec 16, 2025 at 03:08:25PM +0800, Chao Li wrote:
> Given that, I agree we should remove the redundant assignments to
> keep the code clearer and consistent.

Yeah, these could be removed.  I am wondering why 0eb23285a257 did not
bother, but that's no big deal one way or another, just less code at
the end of the day.
--
Michael

Michael Paquier <michael@paquier.xyz> writes:

> On Tue, Dec 16, 2025 at 03:08:25PM +0800, Chao Li wrote:
>> Given that, I agree we should remove the redundant assignments to
>> keep the code clearer and consistent. 
>
> Yeah, these could be removed.  I am wondering why 0eb23285a257 did not
> bother, but that's no big deal one way or another, just less code at
> the end of the day.

A quick grep reveals a bunch of strncpy() calls followed by a '\0'
assignment that could be replaced with strlcpy():

$ rg -A1 strncpy|rg -B1 "= '\\\\0';"
src/interfaces/libpq/fe-secure-openssl.c:        strncpy(buf, conn->sslpassword, size);
src/interfaces/libpq/fe-secure-openssl.c-        buf[size - 1] = '\0';
--
src/bin/pgbench/pgbench.c:        strncpy(*script, option, namelen);
src/bin/pgbench/pgbench.c-        (*script)[namelen] = '\0';
--
doc/src/sgml/ecpg.sgml:            strncpy(name_buf, v.sqlname.data, v.sqlname.length);
doc/src/sgml/ecpg.sgml-            name_buf[v.sqlname.length] = '\0';
--
doc/src/sgml/ecpg.sgml:                strncpy(name_buf, v.sqlname.data, v.sqlname.length);
doc/src/sgml/ecpg.sgml-                name_buf[v.sqlname.length] = '\0';
--
src/interfaces/ecpg/ecpglib/execute.c:                    strncpy(newcopy, (char *) var->value, slen);
src/interfaces/ecpg/ecpglib/execute.c-                    newcopy[slen] = '\0';
--
src/interfaces/ecpg/ecpglib/execute.c:                    strncpy(mallocedval, (char *) var->value, slen);
src/interfaces/ecpg/ecpglib/execute.c-                    mallocedval[slen] = '\0';
--
src/interfaces/ecpg/ecpglib/execute.c:                    strncpy(newcopy, variable->arr, variable->len);
src/interfaces/ecpg/ecpglib/execute.c-                    newcopy[variable->len] = '\0';
--
src/backend/utils/adt/name.c:    strncpy(NameStr(*name), str, NAMEDATALEN);
src/backend/utils/adt/name.c-    NameStr(*name)[NAMEDATALEN - 1] = '\0';

- ilmari

On 16/12/2025 13:16, Dagfinn Ilmari Mannsåker wrote:
> 
> A quick grep reveals a bunch of strncpy() calls followed by a '\0'
> assignment that could be replaced with strlcpy():
> 
> $ rg -A1 strncpy|rg -B1 "= '\\\\0';"
> src/interfaces/libpq/fe-secure-openssl.c:        strncpy(buf, conn->sslpassword, size);
> src/interfaces/libpq/fe-secure-openssl.c-        buf[size - 1] = '\0';

I'm not sure what exactly this code does, but it seems prudent to zero 
the unused bytes since we're dealing with a password.

> --
> src/bin/pgbench/pgbench.c:        strncpy(*script, option, namelen);
> src/bin/pgbench/pgbench.c-        (*script)[namelen] = '\0';

Yeah, this one could use strlcpy(). Or memcpy(). Or pstrndup().

> --
> doc/src/sgml/ecpg.sgml:            strncpy(name_buf, v.sqlname.data, v.sqlname.length);
> doc/src/sgml/ecpg.sgml-            name_buf[v.sqlname.length] = '\0';
> --
> doc/src/sgml/ecpg.sgml:                strncpy(name_buf, v.sqlname.data, v.sqlname.length);
> doc/src/sgml/ecpg.sgml-                name_buf[v.sqlname.length] = '\0';
> --
> src/interfaces/ecpg/ecpglib/execute.c:                    strncpy(newcopy, (char *) var->value, slen);
> src/interfaces/ecpg/ecpglib/execute.c-                    newcopy[slen] = '\0';
> --
> src/interfaces/ecpg/ecpglib/execute.c:                    strncpy(mallocedval, (char *) var->value, slen);
> src/interfaces/ecpg/ecpglib/execute.c-                    mallocedval[slen] = '\0';
> --
> src/interfaces/ecpg/ecpglib/execute.c:                    strncpy(newcopy, variable->arr, variable->len);
> src/interfaces/ecpg/ecpglib/execute.c-                    newcopy[variable->len] = '\0';

I don't know if these depend on the zero-padding...

> --
> src/backend/utils/adt/name.c:    strncpy(NameStr(*name), str, NAMEDATALEN);
> src/backend/utils/adt/name.c-    NameStr(*name)[NAMEDATALEN - 1] = '\0';

This one *does* require the zero-padding, there's a comment that says so:

> void
> namestrcpy(Name name, const char *str)
> {
>     /* NB: We need to zero-pad the destination. */
>     strncpy(NameStr(*name), str, NAMEDATALEN);
>     NameStr(*name)[NAMEDATALEN - 1] = '\0';
> }

- Heikki