On Thu, Apr 29, 2021 at 06:35:28PM +0530, Vaibhav Dalvi wrote: > Hi, > > The function quote_identifier has extra post-increment operation as > highlighted below, > > char * > quote_identifier(const char *s) > { > char *result = pg_malloc(strlen(s) * 2 + 3); > char *r = result; > > *r++ = '"'; > while (*s) > { > if (*s == '"') > *r++ = *s; > *r++ = *s; > s++; > } > *r++ = '"'; > **r++ = '\0';* > > return result; > } > > I think *r = '\0' is enough here. Per precedence table the precedence of > postfix increment operator is higher. The above statement increments 'r' > pointer address but returns the original un-incremented pointer address, > which is then dereferenced. Correct me if I am wrong here. > > If my understanding is correct then '++' is not needed in the > above highlighted statement which is leading to overhead.
I don't think the integer increment during pg_upgrade is a meaningful overhead. You could check the compiler's assembly output it may be the same even without the ++. I'd suggest to leave it as it's currently written, since the idiom on every other line is *r++ = ..., it'd be strange to write it differently here, and could end up being confusing or copied+pasted somewhere else. > Find an attached patch which does the same. This can be backported till v96. In any case, think it would not be backpatched, since it's essentially cosmetic. > diff --git a/src/bin/pg_upgrade/util.c b/src/bin/pg_upgrade/util.c > index fc20472..dc000d0 100644 > --- a/src/bin/pg_upgrade/util.c > +++ b/src/bin/pg_upgrade/util.c > @@ -198,7 +198,7 @@ quote_identifier(const char *s) > s++; > } > *r++ = '"'; > - *r++ = '\0'; > + *r = '\0'; > > return result; > }