Re: Remove useless casting to the same type

On Mon, Nov 24, 2025 at 2:26 AM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
> Attached is a patch to $SUBJECT.

> --- a/src/backend/access/gin/gindatapage.c
> +++ b/src/backend/access/gin/gindatapage.c
> @@ -607,11 +607,11 @@ dataBeginPlaceToPageLeaf(GinBtree btree, Buffer buf, GinBtreeStack *stack,
>
>   if (append)
>   elog(DEBUG2, "appended %d new items to block %u; %d bytes (%d to go)",
> - maxitems, BufferGetBlockNumber(buf), (int) leaf->lsize,
> + maxitems, BufferGetBlockNumber(buf), leaf->lsize,
>   items->nitem - items->curitem - maxitems);

Hm. How do we feel, as a group, about superstitious casts in variadic
calls? I don't feel like they're in the same class as the other fixes.

Argument for: it's nice to know at a glance that a printf() invocation
won't corrupt something horribly, especially if I'm quickly scanning
code during a CVE analysis, and especially if the variable is named as
if it could maybe be a size_t. Do our compilers warn us well enough
now, in practice?

Argument against: it takes up precious columns and focuses attention
away from other things. Like the fact that (items->nitem -
items->curitem - maxitems) is unsigned and printed as signed here. :D
Maybe we should make the code compile cleanly under
-Wformat-signedness at some point... (not in this patch, not signing
you up for that)

> @@ -389,7 +389,7 @@ hash_xlog_split_allocate_page(XLogReaderState *record)
>
>   /* extract low and high masks. */
>   memcpy(&lowmask, data, sizeof(uint32));
> - highmask = (uint32 *) ((char *) data + sizeof(uint32));
> + highmask = (uint32 *) (data + sizeof(uint32));

I wonder about these, too. I like knowing what the code does without
having to check the type of `data`. But then later on we do a `data +=
sizeof(uint32) * 2`, so you have to check the type anyway, so... I
don't know.

> +++ b/contrib/fuzzystrmatch/dmetaphone.c
> @@ -327,7 +327,7 @@ GetAt(metastring *s, int pos)
>   if ((pos < 0) || (pos >= s->length))
>   return '\0';
>
> - return ((char) *(s->str + pos));
> + return *(s->str + pos);

Isn't this just s->str[pos]? Ditto for SetAt(), right afterwards.

--Jacob