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 |
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 |