Re: Compiler warnings in psqloodbc 08.03.0200

Hi Zoltan-san.

I appreciate much work. Please let me late review by the reason for not having margin
time now. sorry. However, we did those checks and adjustments in much environment.
They are VISTA-32bit, winXP-32bit, win2008-64bit,RedHat x86_64, FreeBSD-32bit,
Furthermore, VC6, VC8, Access2000,Access-XP,2005, Cygwin and etc required serious time....
Anyway, As for the Ver 08.03.0300, the result of those tests is reflected.

BTW, the description document point of nptl was unknown...
--
> 2) Fix for "implicit declaration of pthread_mutexattr_settype"
> on my Fedora 9 system.
--

Thanks.

Regards,
Hiroshi Saito

----- Original Message -----
From: "Zoltan Boszormenyi" <zb(at)cybertec(dot)at>

> Hi,
>
> here are two patches that are applicable after my previous patch.
> 1) Cleanup to replace CSTR func = "..." with __FUNCTION__.
> There will be no more "'func' is defined but not used."
> This was done by sed scripts and verified manually
> so only the relevant places were substituted.
> 2) Fix for "implicit declaration of pthread_mutexattr_settype"
> on my Fedora 9 system.
>
> Adding "-Wno-pointer-sign" to AM_CFLAGS would
> prevent "pointer differ in signedness" warnings but
> it would cover an error in erroneous 8-bit arithmetics.
> Cleaning up the SQLCHAR vs. char problems is preferred.
>
> Best regards,
> Zoltán Böszörményi
>
> Zoltan Boszormenyi írta:
>> 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/
>
>

--------------------------------------------------------------------------------

> diff -durpN psqlodbc-08.03.0300.new1/Makefile.am psqlodbc-08.03.0300.new2/Makefile.am
> --- psqlodbc-08.03.0300.new1/Makefile.am 2008-09-30 13:14:09.000000000 +0200
> +++ psqlodbc-08.03.0300.new2/Makefile.am 2008-09-30 13:36:31.000000000 +0200
> @@ -15,8 +15,8 @@ lib_LTLIBRARIES = psqlodbcw.la
> else
> lib_LTLIBRARIES = psqlodbca.la
> endif
> -
> -AM_CFLAGS = -Wall
> +
> +AM_CFLAGS = -Wall -D_BSD_SOURCE -D_XOPEN_SOURCE=500
>
> AM_LDFLAGS = -module -no-undefined -avoid-version
>
>