On 2021-Jul-16, David Rowley wrote:
> On Fri, 16 Jul 2021 at 20:35, Japin Li <[email protected]> wrote:
> > > When I fix a bug about ALTER SUBSCRIPTION ... SET (slot_name) [1], Ranier
> > > Vilela
> > > finds that ReplicationSlotValidateName() has redundant strlen() call,
> > > Since it's
> > > not related to that problem, so I start a new thread to discuss it.
>
> I think this is a waste of time. The first strlen() call is just
> checking for an empty string. I imagine all compilers would just
> optimise that to checking if the first char is '\0';
I could find the following idioms
95 times: var[0] == '\0'
146 times: *var == '\0'
35 times: strlen(var) == 0
Resp.
git grep "[a-zA-Z_]*\[0\] == '\\\\0"
git grep "\*[a-zA-Z_]* == '\\\\0"
git grep "strlen([^)]*) == 0"
See https://postgr.es/m/[email protected] about replacing
strlen with a check on first byte being zero. So still not Ranier's
patch, but rather the attached. I doubt this change is worth committing
on its own, though, since performance-wise it doesn't matter at all; if
somebody were to make it so that all "strlen(foo) == 0" occurrences were
changed to use the test on byte 0, that could be said to be establishing
a consistent style, which might be more pallatable.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Just treat us the way you want to be treated + some extra allowance
for ignorance." (Michael Brusser)
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 33b85d86cc..d56f1b76bc 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -173,7 +173,7 @@ ReplicationSlotValidateName(const char *name, int elevel)
{
const char *cp;
- if (strlen(name) == 0)
+ if (name[0] == '\0')
{
ereport(elevel,
(errcode(ERRCODE_INVALID_NAME),