Re: Compiler warnings in psqloodbc 08.03.0200 - Mailing list pgsql-odbc
From | Zoltan Boszormenyi |
---|---|
Subject | Re: Compiler warnings in psqloodbc 08.03.0200 |
Date | |
Msg-id | 48E25A35.2010001@cybertec.at Whole thread Raw |
In response to | Re: Compiler warnings in psqloodbc 08.03.0200 ("Hiroshi Saito" <z-saito@guitar.ocn.ne.jp>) |
List | pgsql-odbc |
Hi, Hiroshi Saito írta: > Hi Zoltan-san. > > I appreciate much work. Please let me late review by the reason for > not having margin > time now. No problem, take your time. I just wanted it to be out of my machine. :-) > 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. I understand your position. I just checked on Visual C++ 2005 and 2008 and they also understand __FUNCTION__ and it works as expected, i.e. it presents the current function name just as on GCC. The Microsoft OS doesn't matter, it's the compiler that matters. I suspect things like __FUNCTION__ are actually defined in the C language standard as a mandatory feature provided by the compiler and *BSDs also conform. However, on some systems (newer Linux systems with a recent GCC 4.x) spit out "pointers differ in signedness" warnings if the signedness of char variables/expressions differ in assignments or passed-in vs declared function parameters. As Tom said, this can create a situation where there are so many of them that other more serious warnings are simply lost in the noise. A zero-warning policy is indeed useful in C language projects. > BTW, the description document point of nptl was unknown... > -- The below is not about NPTL workings, it's about GLIBC in general on Fedora 9 and most other GLIBC-using systems, like your RedHat x86_64 above. In pthread.h, pthread_mutexattr_settype() and others are protected inside #ifdef __USE_UNIX98 ... #endif So simply compiling it produces a "implicit declaration of //function '...'" warning. __USE_UNIX98 gets defined in features.h if the symbol _XOPEN_SOURCE is defined with a value >=500. features.h is included by all other GLIBC headers directly or indirectly. But defining _XOPEN_SOURCE seems to override the GCC default (POSIX, XOPEN, etc) conformance level symbols and now strcasecmp(), strncasecmp(), snprintf() and malloc() are undefined now. To make them visible again, _BSD_SOURCE or _GNU_SOURCE has to be defined. I chose _BSD_SOURCE because it will not offend people using BSDs. :-) >> 2) Fix for "implicit declaration of pthread_mutexattr_settype" >> on my Fedora 9 system. > -- Best regards, Zoltán Böszörményi > Thanks. > > Regards, > Hiroshi Saito > > ----- Original Message ----- From: "Zoltan Boszormenyi" <zb@cybertec.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@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/ >> >> > > > -------------------------------------------------------------------------------- > > > >> 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 >> >> > > -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/
pgsql-odbc by date: