Re: Compiler warnings in psqloodbc 08.03.0200 - Mailing list pgsql-odbc

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@sss.pgh.pa.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/