Thread: msvc const warnings

MSVC generates a number of const warnings, that's been discussed before
(http://archives.postgresql.org/pgsql-hackers/2007-01/msg01214.php).

Attached patch adds a pragma to get rid of the warnings. Again, not sure if
it's worth it?

I think I need to #ifdef the pragma - gcc generates a warning if it sees it
in -Wall, and that'd certainly be worse :-)

Stefan mentioned that the warning may be one that shows up in a different
compiler somewhere as well, thouh, which might indicate that we should fix
the underlying issue? (Even if the code is correct, if it confuses multiple
compilers...)

//Magnus

"Magnus Hagander" <magnus@hagander.net> writes:

> Stefan mentioned that the warning may be one that shows up in a different
> compiler somewhere as well, thouh, which might indicate that we should fix
> the underlying issue? (Even if the code is correct, if it confuses multiple
> compilers...)

I think the right fix is just to remove the const qualifier. It's clearly not
treating the pointer as const if it's passing it to pfree which is surely a
state change if anything is.

We do far too much casting to (void*) as it is and I would dearly love to go
through the whole source tree and remove all the redundant casts. All they do
is cover up real type clash bugs.

--
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com

Gregory Stark <stark@enterprisedb.com> writes:
> "Magnus Hagander" <magnus@hagander.net> writes:
>> Stefan mentioned that the warning may be one that shows up in a different
>> compiler somewhere as well, thouh, which might indicate that we should fix
>> the underlying issue? (Even if the code is correct, if it confuses multiple
>> compilers...)

> I think the right fix is just to remove the const qualifier. It's clearly not
> treating the pointer as const if it's passing it to pfree which is surely a
> state change if anything is.

That was what you claimed in the previous discussion, but you were wrong
then and you're still wrong.  The pointer is "const char **" which means
it is a pointer to some pointers that are not themselves constant.  That
is, the palloc'd area *contains* pointers to const strings, but that
doesn't make the palloc'd area itself const.  You're making the same
mistake msvc does.

I agree though that the #pragma solution is awfully ugly.  What I'd
be inclined to do is

    /* cast away indirect const to avoid warnings from broken compilers */
    free((void *) headers);
    ...


            regards, tom lane

"Tom Lane" <tgl@sss.pgh.pa.us> writes:

> Gregory Stark <stark@enterprisedb.com> writes:
>> "Magnus Hagander" <magnus@hagander.net> writes:
>>> Stefan mentioned that the warning may be one that shows up in a different
>>> compiler somewhere as well, thouh, which might indicate that we should fix
>>> the underlying issue? (Even if the code is correct, if it confuses multiple
>>> compilers...)
>
>> I think the right fix is just to remove the const qualifier. It's clearly not
>> treating the pointer as const if it's passing it to pfree which is surely a
>> state change if anything is.
>
> That was what you claimed in the previous discussion, but you were wrong
> then and you're still wrong.  The pointer is "const char **" which means
> it is a pointer to some pointers that are not themselves constant.

Oh, wow. You're right on both counts. I didn't find my own comment on the old
thread before responding. And I didn't notice it was a const char **. That is
a crazy bug.

Can we just disable const checking for MSVCC in general without using the
#pragmas? It clearly doesn't understand how const works making that warning
from it useless.

--
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com

Gregory Stark <stark@enterprisedb.com> writes:
> Can we just disable const checking for MSVCC in general without using the
> #pragmas? It clearly doesn't understand how const works making that warning
> from it useless.

+1 ... any useful warning of this kind will be had from other compilers.

            regards, tom lane

Am Dienstag, 24. Juli 2007 17:24 schrieb Gregory Stark:
> Can we just disable const checking for MSVCC in general without using the
> #pragmas? It clearly doesn't understand how const works making that warning
>
> >from it useless.

That was my thought.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Tom Lane wrote:
> Gregory Stark <stark@enterprisedb.com> writes:
>> Can we just disable const checking for MSVCC in general without using the
>> #pragmas? It clearly doesn't understand how const works making that warning
>> from it useless.
>
> +1 ... any useful warning of this kind will be had from other compilers.

Sure, there are a couple of places we can do it:
1) We can add the pragma at the top of the file, which will turn it off
for that file, but keep it for other files.
2) We can turn it off on a per-project basis, meaning we turn it off for
the entire backend and the entire psql, but not for other EXEs and DLLs.
3) We can just turn it off for everything

Note that it'll of course turn off the warning in *all* cases, not just
the "free const chra **"-one.

Which way would be preferred?

//Magnus

Magnus Hagander wrote:
> Tom Lane wrote:
>
>> Gregory Stark <stark@enterprisedb.com> writes:
>>
>>> Can we just disable const checking for MSVCC in general without using the
>>> #pragmas? It clearly doesn't understand how const works making that warning
>>> from it useless.
>>>
>> +1 ... any useful warning of this kind will be had from other compilers.
>>
>
> Sure, there are a couple of places we can do it:
> 1) We can add the pragma at the top of the file, which will turn it off
> for that file, but keep it for other files.
> 2) We can turn it off on a per-project basis, meaning we turn it off for
> the entire backend and the entire psql, but not for other EXEs and DLLs.
> 3) We can just turn it off for everything
>
> Note that it'll of course turn off the warning in *all* cases, not just
> the "free const chra **"-one.
>
> Which way would be preferred?
>
>
>

Since it's apparently useless anyway, I vote for everywhere.

cheers

andrew

Magnus Hagander <magnus@hagander.net> writes:
> Tom Lane wrote:
>> Gregory Stark <stark@enterprisedb.com> writes:
>>> Can we just disable const checking for MSVCC in general without using the
>>> #pragmas? It clearly doesn't understand how const works making that warning
>>> from it useless.
>>
>> +1 ... any useful warning of this kind will be had from other compilers.

> Sure, there are a couple of places we can do it:
> 1) We can add the pragma at the top of the file, which will turn it off
> for that file, but keep it for other files.
> 2) We can turn it off on a per-project basis, meaning we turn it off for
> the entire backend and the entire psql, but not for other EXEs and DLLs.
> 3) We can just turn it off for everything

> Note that it'll of course turn off the warning in *all* cases, not just
> the "free const chra **"-one.

> Which way would be preferred?

Turning it off globally is what I had in mind, and I assume Greg too.
The point is that if MSVC gets this case wrong, then it probably gets
many other cases wrong, and we shouldn't have to work around them one
at a time.

            regards, tom lane

Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> Tom Lane wrote:
>>> Gregory Stark <stark@enterprisedb.com> writes:
>>>> Can we just disable const checking for MSVCC in general without using the
>>>> #pragmas? It clearly doesn't understand how const works making that warning
>>>> from it useless.
>>> +1 ... any useful warning of this kind will be had from other compilers.
>
>> Sure, there are a couple of places we can do it:
>> 1) We can add the pragma at the top of the file, which will turn it off
>> for that file, but keep it for other files.
>> 2) We can turn it off on a per-project basis, meaning we turn it off for
>> the entire backend and the entire psql, but not for other EXEs and DLLs.
>> 3) We can just turn it off for everything
>
>> Note that it'll of course turn off the warning in *all* cases, not just
>> the "free const chra **"-one.
>
>> Which way would be preferred?
>
> Turning it off globally is what I had in mind, and I assume Greg too.
> The point is that if MSVC gets this case wrong, then it probably gets
> many other cases wrong, and we shouldn't have to work around them one
> at a time.

ok. I'll do it globally when I'm back at a box where I can rebuild the
thing fast enough not to get bored :-) Meaning tomorrow.

//Magnus

On Tue, Jul 24, 2007 at 09:43:51PM +0200, Magnus Hagander wrote:
> Tom Lane wrote:
> > Magnus Hagander <magnus@hagander.net> writes:
> >> Tom Lane wrote:
> >>> Gregory Stark <stark@enterprisedb.com> writes:
> >>>> Can we just disable const checking for MSVCC in general without using the
> >>>> #pragmas? It clearly doesn't understand how const works making that warning
> >>>> from it useless.
> >>> +1 ... any useful warning of this kind will be had from other compilers.
> >
> >> Sure, there are a couple of places we can do it:
> >> 1) We can add the pragma at the top of the file, which will turn it off
> >> for that file, but keep it for other files.
> >> 2) We can turn it off on a per-project basis, meaning we turn it off for
> >> the entire backend and the entire psql, but not for other EXEs and DLLs.
> >> 3) We can just turn it off for everything
> >
> >> Note that it'll of course turn off the warning in *all* cases, not just
> >> the "free const chra **"-one.
> >
> >> Which way would be preferred?
> >
> > Turning it off globally is what I had in mind, and I assume Greg too.
> > The point is that if MSVC gets this case wrong, then it probably gets
> > many other cases wrong, and we shouldn't have to work around them one
> > at a time.
>
> ok. I'll do it globally when I'm back at a box where I can rebuild the
> thing fast enough not to get bored :-) Meaning tomorrow.

Done.

//Magnus