On Thu, Nov 14, 2024 at 09:59:07AM +0100, Peter Eisentraut wrote:
> On 29.10.24 15:20, Tom Lane wrote:
> > Peter Eisentraut <peter@eisentraut.org> writes:
> > > There are a bunch of (void *) casts in the code that don't make sense to
> > > me. I think some of these were once necessary because char * was used
> > > in place of void * for some function arguments. And some of these were
> > > probably just copied around without further thought. I went through and
> > > cleaned up most of these. I didn't find any redeeming value in these.
> > > They are just liable to hide actual problems such as incompatible types.
> > > But maybe there are other opinions.
> >
> > I don't recall details, but I'm fairly sure some of these prevented
> > compiler warnings on some (old?) compilers. Hard to be sure if said
> > compilers are all gone.
> >
> > Looking at the sheer size of the patch, I'm kind of -0.1, just
> > because I'm afraid it's going to create back-patching gotchas.
> > I don't really find that it's improving readability, though
> > clearly that's a matter of opinion.
>
> I did a bit of archeological research on these. None of these casts were
> ever necessary, and in many cases even the original patch that introduced an
> API used the coding style inconsistently. So I'm very confident that there
> are no significant backward compatibility or backpatching gotchas here.
>
> I'm more concerned that many of these just keep getting copied around
> indiscriminately, and this is liable to hide actual type mismatches or
> silently discard qualifiers. So I'm arguing in favor of a more restrictive
> style in this matter.
I agree. I realize this will cause backpatch complexities, but I think
removing these will be a net positive.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
When a patient asks the doctor, "Am I going to die?", he means
"Am I going to die soon?"
