Thread: Getting rid of warnings
Attached patch gets rid of most of the remaining warnings on a VC++
build. Summary is:
* A bunch of places that had different const specifyer in the header and
in the body of the function. (contrib/intarray, src/timezone)
* 1.2 and such constants are double and cause warning. Define as floats
(contrib/pg_trgm and contrib/tsearch2)
* HAVE_STRERROR is defined by python, so only conditionally redefine it
in pg_config.h
* NULL function pointer in SSL call cast to the correct pointer type
* ssize_t is defined in pg_config_os.h, remove from libpq-int.h
* Always skip warning 4102 ("label nnn: unreferenced label") caused by
bison.
* Support for ignoring linker warnings, and ignore the warning about
PRIVATE on DllRegisterServer. Can't fix properly because PRIVATE is
not supported by mingw.
Attachment
Magnus Hagander wrote: > * NULL function pointer in SSL call cast to the correct pointer type Why not write NULL? In the alternative, declare the variable to have the right type to begin with. -- Peter Eisentraut http://developer.postgresql.org/~petere/
On Thu, Jan 25, 2007 at 03:29:50PM +0100, Peter Eisentraut wrote: > Magnus Hagander wrote: > > * NULL function pointer in SSL call cast to the correct pointer type > > Why not write NULL? > > In the alternative, declare the variable to have the right type to begin > with. I went down the path of least resistance :-) Not sure if there was a reason to code it that way from the beginning. NULL works equally well (tested). I assume this change is something ac ommitter can take care of at commit-time, so I'll not resubmit for it unless asked to. //Magnus
Magnus Hagander <magnus@hagander.net> writes:
> bool
> ! isort(int4 *a, int len)
> {
> bool
> ! isort(int4 *a, const int len)
> {
If VC thinks that that is required to fix a warning, it's too broken to live.
AFAICS what you've got there is a compiler that is being pedantically
strict about language details that it does not actually understand well;
its apparent idea that "const **" means "const * const *" being much in
the same line as this one.
I don't mind the non-const-related parts of this patch, but I recommend
rejecting all the const-decoration changes.
regards, tom lane
On Thu, Jan 25, 2007 at 10:57:29AM -0500, Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
> > bool
> > ! isort(int4 *a, int len)
> > {
> > bool
> > ! isort(int4 *a, const int len)
> > {
>
> If VC thinks that that is required to fix a warning, it's too broken to live.
> AFAICS what you've got there is a compiler that is being pedantically
> strict about language details that it does not actually understand well;
> its apparent idea that "const **" means "const * const *" being much in
> the same line as this one.
Not sure I understand.
The header had:
isort(int4 *a, const int len)
and the code had
isort(int4 *a, int len)
Where does the ** part come in there? It's not even a pointer!
I see the ** part for the free()/pfree() issues, but the one you're
quoting there is completely different. This is, AFAICS, a case of the
header not matching the body.
I'm fine with having that one rejected as well, and weill just file that
away as an expected-please-ignore warning, but I'd prefer to understand
why :)
//Magnus
On Thu, 2007-01-25 at 17:11 +0100, Magnus Hagander wrote: > The header had: > isort(int4 *a, const int len) > and the code had > isort(int4 *a, int len) ISTM that the "const" keyword to an "int" function argument is pointless, so the right fix is to remove the "const" from the declaration in the header. > Where does the ** part come in there? It's not even a pointer! I believe Tom was just referencing that as an example of a less-than-perfect compiler warning from VC++. -Neil
Magnus Hagander <magnus@hagander.net> writes:
> Not sure I understand.
> The header had:
> isort(int4 *a, const int len)
> and the code had
> isort(int4 *a, int len)
Oh, I see. Yeah, that's inconsistent, though my thought would be to
remove the (rather useless) const decoration in the header. I believe
this coding is actually legal per C99, though, precisely because the
version in the header is only decoration --- callers of the function
do not care whether it thinks the parameter value is immutable inside
itself. The one at the function definition site is what counts ...
regards, tom lane
I need an updated version of this to apply. The suggested changes are
too extensive.
---------------------------------------------------------------------------
Magnus Hagander wrote:
> Attached patch gets rid of most of the remaining warnings on a VC++
> build. Summary is:
> * A bunch of places that had different const specifyer in the header and
> in the body of the function. (contrib/intarray, src/timezone)
> * 1.2 and such constants are double and cause warning. Define as floats
> (contrib/pg_trgm and contrib/tsearch2)
> * HAVE_STRERROR is defined by python, so only conditionally redefine it
> in pg_config.h
> * NULL function pointer in SSL call cast to the correct pointer type
> * ssize_t is defined in pg_config_os.h, remove from libpq-int.h
> * Always skip warning 4102 ("label nnn: unreferenced label") caused by
> bison.
> * Support for ignoring linker warnings, and ignore the warning about
> PRIVATE on DllRegisterServer. Can't fix properly because PRIVATE is
> not supported by mingw.
>
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo@postgresql.org so that your
> message can get through to the mailing list cleanly
--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Bruce Momjian wrote: > I need an updated version of this to apply. The suggested changes are > too extensive. I'll try to do this tomorrow. If I get it right, the changes needed are: NULL instead of cast of function ptr, per Peter. Do the const-change in the other direction in contrib/intarray. The patch never contained anything for those const ** warnings, so I'll just continue not to include those. If I missed anything, let me know :-) //Magnus
Tom Lane wrote: > Magnus Hagander <magnus@hagander.net> writes: >> Not sure I understand. >> The header had: >> isort(int4 *a, const int len) >> and the code had >> isort(int4 *a, int len) > > Oh, I see. Yeah, that's inconsistent, though my thought would be to > remove the (rather useless) const decoration in the header. I believe > this coding is actually legal per C99, though, precisely because the > version in the header is only decoration --- callers of the function > do not care whether it thinks the parameter value is immutable inside > itself. The one at the function definition site is what counts ... If it wasn't legal, it should be an error and not a warning. I would guess it's a warning simply so you should be aware that it's ignored. I'll update the patch for it. //Magnus
On Thu, Jan 25, 2007 at 08:56:27PM +0100, Magnus Hagander wrote: > Bruce Momjian wrote: > > I need an updated version of this to apply. The suggested changes are > > too extensive. > > > I'll try to do this tomorrow. If I get it right, the changes needed are: > NULL instead of cast of function ptr, per Peter. > Do the const-change in the other direction in contrib/intarray. > > The patch never contained anything for those const ** warnings, so I'll > just continue not to include those. > Ok, here's an updated patch per this. //Magnus
Attachment
On Fri, 2007-01-26 at 12:53 +0100, Magnus Hagander wrote: > Ok, here's an updated patch per this. Applied -- thanks for the patch. -Neil