Re: Remove useless casts to (void *)

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?"