Thread: implicit casts from void*
I received on off-list report that commit e2809e3a101 causes an error when building an extension written in C++, since $subject is in a header file. The fix is simply to add an explicit cast, so I plan to push the attached soon. Bikeshedding: We could additionally change the pg_crc*.c files to make them consistent, but I have not done that yet. It seems we prefer explicit casts anyway but don't enforce that. -- John Naylor Amazon Web Services
Attachment
John Naylor <johncnaylorls@gmail.com> writes:
> I received on off-list report that commit e2809e3a101 causes an error
> when building an extension written in C++, since $subject is in a
> header file. The fix is simply to add an explicit cast, so I plan to
> push the attached soon.
Hmpfh. No objection to your patch, but I wonder why
"headerscheck --cplusplus" didn't find this? Can we get it
to do so?
> Bikeshedding: We could additionally change the pg_crc*.c files to make
> them consistent, but I have not done that yet. It seems we prefer
> explicit casts anyway but don't enforce that.
Meh. There are an awful lot of places where we assume such casts
are okay. I'm willing to adopt a stricter definition in header
files, but it feels like requiring it in .c files is useless
make-work. As a perhaps not quite exact parallel, we mostly
don't object to writing
if (ptr)
as a shortcut for
if (ptr != NULL)
though it's hard to see the former as anything but an implicit
cast to boolean.
regards, tom lane
On Tue, Jul 1, 2025 at 10:36 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > John Naylor <johncnaylorls@gmail.com> writes: > > I received on off-list report that commit e2809e3a101 causes an error > > when building an extension written in C++, since $subject is in a > > header file. The fix is simply to add an explicit cast, so I plan to > > push the attached soon. > > Hmpfh. No objection to your patch, but I wonder why > "headerscheck --cplusplus" didn't find this? Can we get it > to do so? Good question, and it turns out it catches it just fine, but you have to configure with CPPFLAGS="-msse4.2" (or run the script on a Red Hat 9-ish system). -- John Naylor Amazon Web Services
John Naylor <johncnaylorls@gmail.com> writes:
> On Tue, Jul 1, 2025 at 10:36 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Hmpfh. No objection to your patch, but I wonder why
>> "headerscheck --cplusplus" didn't find this? Can we get it
>> to do so?
> Good question, and it turns out it catches it just fine, but you have
> to configure with CPPFLAGS="-msse4.2" (or run the script on a Red Hat
> 9-ish system).
Ha, indeed you are right. On my RHEL9 box, it's kinda drowned out
by complaints about
/usr/include/c++/11/bits/range_access.h:109:3: error: template with C linkage
109 | template<typename _Tp> _Tp* end(valarray<_Tp>&) noexcept;
| ^~~~~~~~
/tmp/headerscheck.u5CrRM/test.cpp:1:1: note: ‘extern "C"’ linkage started here
1 | extern "C" {
| ^~~~~~~~~~
but looking closer, I do see some
./src/include/port/pg_crc32c.h: In function ‘pg_crc32c pg_comp_crc32c_dispatch(pg_crc32c, const void*, size_t)’:
./src/include/port/pg_crc32c.h:75:42: error: invalid conversion from ‘const void*’ to ‘const unsigned char*’
[-fpermissive]
75 | const unsigned char *p = data;
| ^~~~
| |
| const void*
regards, tom lane
On 02.07.25 09:01, John Naylor wrote:
> On Tue, Jul 1, 2025 at 9:24 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Ha, indeed you are right. On my RHEL9 box, it's kinda drowned out
>> by complaints about
>>
>> /usr/include/c++/11/bits/range_access.h:109:3: error: template with C linkage
>> 109 | template<typename _Tp> _Tp* end(valarray<_Tp>&) noexcept;
>> | ^~~~~~~~
>> /tmp/headerscheck.u5CrRM/test.cpp:1:1: note: ‘extern "C"’ linkage started here
>> 1 | extern "C" {
>> | ^~~~~~~~~~
>
> After pushing my fix, I looked into this, and CI works around this by
> disabling ICU. A proper fix was discussed here, but it trailed off:
>
>
https://www.postgresql.org/message-id/flat/20230311033727.koa4saxy5wyquu6s%40awork3.anarazel.de#03346c63050bbc69dfca8981a5698e4a
>
> I came up with the attached -- Andres, Peter, does this match your recollection?
This looks sensible to me. Assuming that it works for this purpose, it
seems otherwise harmless.