On Thu, Nov 14, 2024 at 09:59:07AM +0100, Peter Eisentraut wrote: > On 29.10.24 15:20, Tom Lane wrote: > > Peter Eisentraut <pe...@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 <br...@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?"
