Re: Compiler warnings in psqloodbc 08.03.0200

From: Zoltan Boszormenyi <zb(at)cybertec(dot)at>
To: Hiroshi Saito <z-saito(at)guitar(dot)ocn(dot)ne(dot)jp>
Cc: pgsql-odbc(at)postgresql(dot)org, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Hans-Juergen Schoenig <hs(at)cybertec(dot)at>
Subject: Re: Compiler warnings in psqloodbc 08.03.0200
Date: 2008-09-29 10:05:42
Message-ID: 48E0A876.10006@cybertec.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-odbc

Oops, the actual fix is attached.

Zoltan Boszormenyi írta:
> Hi,
>
> here's the fix for all non-pointer-signedness warnings,
> against 08.03.0300 that was released meanwhile. Now
> the compilation only emits 246 "differ in signedness"
> warnings, which is still too much noise. I agree with
> Tom Lane that those should be cleaned up if for nothing
> else, than the real bugs don't get lost in the noise.
>
> The patch cleans up such warnings in several files:
> "label 'cleanup' defined but not used" and
> "unused variable 'func'"
>
> In convert.c::convert_escape():
> "'param_consumed' may be used uninitialized in this function"
> The variable "param_consumed" was not assigned a value
> in all cases by processParameters() upon returning,
> so I tried to fix it there. Now it does, please review.
>
> In descriptor.c::TI_Constructor():
> "the address of 'qual' will always evaluate as 'true'"
> STRX_TO_NAME() has to be used instead of STR_TO_NAME.
>
> In drvconn.c::dconn_get_attributes():
> "'last' may be used uninitialized in this function"
> The first parameter to strtok_r() may be NULL if
> strdup() returns NULL. In that case strtok_r() may
> corrupt unknown memory areas.
>
> In pgapi30.c, two instances of
> "cast from pointer to integer of different size"
>
> In psqlodbc.c()::finalize_global_cs() is only used inside
> "#ifdef WIN32" but was defined outside causing a
> "defined but not used" warning.
>
> Some notes:
> - Instead of using 'CSTR func = "funcname";' everywhere,
> it would be better to use the __FUNCTION__ pre-processor
> macro. This way, the "unused variable 'func'" is eliminated
> once and for all as __FUNCTION__ is available everywhere.
> - The sea of "differ in signedness" warnings are caused by
> the difference of "{SQL|U}CHAR *" and plain "char *".
> Many ODBC API calls expect "SQLCHAR *" input.
> Using strcpy(), strcmp(), etc. on them causes warnings.
> Many internal psqlODBC functions expect a character input
> and they are also used inconsistenly with different
> signed and unsigned parameters.
> Either the API parameters has to be treated internally
> as "char *" and keep a local variable for this purpose,
> or pass the SQLCHAR * down in other functions which
> have to be declared with the appropriate header.
> Fixing it either way would be quite invasive in terms
> of patch size. The latter would mean smaller and
> cleaner C source.
>
> Best regards,
> Zoltán Böszörményi
>
> Hiroshi Saito írta:
>
>> Hi.
>>
>> Surely, we have not made offer sufficient about x86_64.... However,
>> The check of operation
>> was performed by the temporary rental machine. Moreover, there is also
>> a situation of taking
>> time in the check of the present BIGENDIAN. Then, late work may be
>> blamed....
>> Anyway, In order to avoid a problem, to be warning should clear.
>>
>> BTW, FreeBSD x86 is this.
>> inet% gmake socket.o
>> if gcc -DHAVE_CONFIG_H -I. -I. -I. -I/usr/local/include
>> -I/usr/local/pgsql/include -Wall -g -O2 -MT socket.o -MD -MP -MF
>> ".deps/socket.Tpo" -c -o socket.o socket.c; \
>> then mv -f ".deps/socket.Tpo" ".deps/socket.Po"; else rm -f
>> ".deps/socket.Tpo"; exit 1; fi
>> socket.c: In function `SOCK_wait_for_ready':
>> socket.c:468: warning: 'no_timeout' might be used uninitialized in
>> this function
>>
>> Regards,
>> Hiroshi Saito
>>
>> ----- Original Message ----- From: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
>>
>>
>>
>>> I wrote:
>>>
>>>> BTW, isn't anyone paying attention to compiler warnings in this code
>>>> base?
>>>>
>>> To be concrete, attached is a list of warnings I see when building .0200
>>> using gcc -Wall on an x86_64 Fedora 9 machine. If I were in charge of
>>> this project I'd insist on every one of these being cleaned up --- they
>>> are at least evidence of sloppy programming, and a significant fraction
>>> look like they mean certain crashes if the code ever gets executed.
>>>
>>> BTW, I've omitted 261 "pointer targets differ in signedness" warnings...
>>> those are probably not interesting, but I'd still recommend cleaning
>>> them up, if only so that more important warnings don't get lost in the
>>> noise. The core Postgres project has maintained a zero-tolerance policy
>>> on gcc warnings for years, and I think it's served us well.
>>>
>>> regards, tom lane
>>>
>>
>
>
>

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/

Attachment Content-Type Size
psqlodbc-08.03.0300-warningfixes.patch text/x-patch 6.1 KB

In response to

Responses

Browse pgsql-odbc by date

  From Date Subject
Next Message Hiroshi Saito 2008-09-30 13:32:46 Re: Compiler warnings in psqloodbc 08.03.0200
Previous Message Zoltan Boszormenyi 2008-09-29 10:03:40 Re: Compiler warnings in psqloodbc 08.03.0200