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 | 48E0A876.10006@cybertec.at Whole thread Raw |
In response to | Re: Compiler warnings in psqloodbc 08.03.0200 (Zoltan Boszormenyi <zb@cybertec.at>) |
List | 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@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.orig/bind.c psqlodbc-08.03.0300/bind.c --- psqlodbc-08.03.0300.orig/bind.c 2008-09-22 12:13:05.000000000 +0200 +++ psqlodbc-08.03.0300/bind.c 2008-09-29 10:25:18.000000000 +0200 @@ -620,7 +620,9 @@ reset_a_iparameter_binding(IPDFields *se int CountParameters(const StatementClass *self, Int2 *inputCount, Int2 *ioCount, Int2 *outputCount) { +#if 0 CSTR func = "CountParameters"; +#endif IPDFields *ipdopts = SC_get_IPDF(self); int i, num_params, valid_count; diff -durpN psqlodbc-08.03.0300.orig/convert.c psqlodbc-08.03.0300/convert.c --- psqlodbc-08.03.0300.orig/convert.c 2008-09-25 14:25:03.000000000 +0200 +++ psqlodbc-08.03.0300/convert.c 2008-09-29 09:45:04.000000000 +0200 @@ -4157,18 +4157,23 @@ processParameters(QueryParse *qp, QueryB size_t *output_count, SQLLEN param_pos[][2]) { CSTR func = "processParameters"; - int retval, innerParenthesis, param_count; + int retval, innerParenthesis, param_count, return_count; BOOL stop; /* begin with outer '(' */ innerParenthesis = 0; param_count = 0; + return_count = 0; stop = FALSE; for (; F_OldPos(qp) < qp->stmt_len; F_OldNext(qp)) { retval = inner_process_tokens(qp, qb); if (retval == SQL_ERROR) + { + if (output_count) + *output_count = return_count; return retval; + } if (ENCODE_STATUS(qp->encstr) != 0) continue; if (qp->in_identifier || qp->in_literal || qp->in_escape) @@ -4203,8 +4208,7 @@ processParameters(QueryParse *qp, QueryB param_pos[param_count][0] = param_pos[param_count][1] = -1; } - if (output_count) - *output_count = F_NewPos(qb); + return_count = F_NewPos(qb); break; case '}': @@ -4215,6 +4219,8 @@ processParameters(QueryParse *qp, QueryB if (stop) /* returns with the last } position */ break; } + if (output_count) + *output_count = return_count; if (param_pos[param_count][0] >= 0) { mylog("%s closing ) not found %d\n", func, innerParenthesis); diff -durpN psqlodbc-08.03.0300.orig/descriptor.c psqlodbc-08.03.0300/descriptor.c --- psqlodbc-08.03.0300.orig/descriptor.c 2007-11-26 13:24:10.000000000 +0100 +++ psqlodbc-08.03.0300/descriptor.c 2008-09-29 10:50:49.000000000 +0200 @@ -33,7 +33,7 @@ void TI_Constructor(TABLE_INFO *self, co STR_TO_NAME(self->bestitem, OID_NAME); sprintf(qual, "\"%s\" = %%u", OID_NAME); - STR_TO_NAME(self->bestqual, qual); + STRX_TO_NAME(self->bestqual, qual); TI_set_hasoids(self); TI_set_hasoids_checked(self); } @@ -584,7 +584,9 @@ static struct static PG_ErrorInfo *DC_create_errorinfo(const DescriptorClass *desc) { +#if 0 CSTR func = "DC_create_erroinfo"; +#endif PG_ErrorInfo *error; ConnectionClass *conn; EnvironmentClass *env; diff -durpN psqlodbc-08.03.0300.orig/drvconn.c psqlodbc-08.03.0300/drvconn.c --- psqlodbc-08.03.0300.orig/drvconn.c 2008-09-22 12:13:12.000000000 +0200 +++ psqlodbc-08.03.0300/drvconn.c 2008-09-29 11:10:17.000000000 +0200 @@ -432,6 +432,8 @@ dconn_get_attributes(copyfunc func, cons our_connect_string = strdup(connect_string); strtok_arg = our_connect_string; + if (strtok_arg == NULL) + return; #ifdef FORCE_PASSWORD_DISPLAY mylog("our_connect_string = '%s'\n", our_connect_string); diff -durpN psqlodbc-08.03.0300.orig/environ.c psqlodbc-08.03.0300/environ.c --- psqlodbc-08.03.0300.orig/environ.c 2007-11-26 13:24:10.000000000 +0100 +++ psqlodbc-08.03.0300/environ.c 2008-09-29 10:25:47.000000000 +0200 @@ -531,7 +531,7 @@ EN_Constructor(void) #endif /* WIN32 */ rv = (EnvironmentClass *) malloc(sizeof(EnvironmentClass)); -cleanup: + if (rv) { rv->errormsg = 0; diff -durpN psqlodbc-08.03.0300.orig/info.c psqlodbc-08.03.0300/info.c --- psqlodbc-08.03.0300.orig/info.c 2008-09-25 14:25:05.000000000 +0200 +++ psqlodbc-08.03.0300/info.c 2008-09-29 10:24:34.000000000 +0200 @@ -824,7 +824,6 @@ mylog("CONVERT_FUNCTIONS=" FORMAT_ULEN " if (pcbInfoValue) *pcbInfoValue = (SQLSMALLINT) len; -cleanup: return result; } @@ -969,7 +968,6 @@ inolog("serial in\n"); } } -cleanup: #undef return /* * also, things need to think that this statement is finished so the @@ -3484,7 +3482,7 @@ PGAPI_ColumnPrivileges( extend_column_bindings(SC_get_ARDF(stmt), 8); /* set up the current tuple pointer for SQLFetch */ result = SQL_SUCCESS; -cleanup: + /* set up the current tuple pointer for SQLFetch */ stmt->status = STMT_FINISHED; stmt->currTuple = -1; diff -durpN psqlodbc-08.03.0300.orig/pgapi30.c psqlodbc-08.03.0300/pgapi30.c --- psqlodbc-08.03.0300.orig/pgapi30.c 2008-09-25 14:25:06.000000000 +0200 +++ psqlodbc-08.03.0300/pgapi30.c 2008-09-29 10:54:39.000000000 +0200 @@ -398,7 +398,9 @@ PGAPI_GetConnectAttr(HDBC ConnectionHand SQLINTEGER Attribute, PTR Value, SQLINTEGER BufferLength, SQLINTEGER *StringLength) { +#if 0 CSTR func = "PGAPI_GetConnectAttr"; +#endif ConnectionClass *conn = (ConnectionClass *) ConnectionHandle; RETCODE ret = SQL_SUCCESS; SQLINTEGER len = 4; @@ -1664,7 +1666,7 @@ PGAPI_SetConnectAttr(HDBC ConnectionHand unsupported = TRUE; break; default: - ret = PGAPI_SetConnectOption(ConnectionHandle, (SQLUSMALLINT) Attribute, (SQLLEN) Value); + ret = PGAPI_SetConnectOption(ConnectionHandle, (SQLUSMALLINT) Attribute, CAST_PTR(SQLLEN, Value)); } if (unsupported) { @@ -1870,7 +1872,7 @@ inolog("set ard=%p\n", stmt->ard); SC_get_ARDF(stmt)->size_of_rowset = CAST_UPTR(SQLULEN, Value); break; default: - return PGAPI_SetStmtOption(StatementHandle, (SQLUSMALLINT) Attribute, (SQLULEN) Value); + return PGAPI_SetStmtOption(StatementHandle, (SQLUSMALLINT) Attribute, CAST_PTR(SQLULEN, Value)); } return ret; } diff -durpN psqlodbc-08.03.0300.orig/psqlodbc.c psqlodbc-08.03.0300/psqlodbc.c --- psqlodbc-08.03.0300.orig/psqlodbc.c 2008-05-11 20:42:24.000000000 +0200 +++ psqlodbc-08.03.0300/psqlodbc.c 2008-09-29 11:32:20.000000000 +0200 @@ -92,6 +92,7 @@ int initialize_global_cs(void) return 0; } +#ifdef WIN32 static void finalize_global_cs(void) { DELETE_COMMON_CS; @@ -104,7 +105,6 @@ static void finalize_global_cs(void) #endif /* _DEBUG */ } -#ifdef WIN32 HINSTANCE NEAR s_hModule; /* Saved module handle. */ /* This is where the Driver Manager attaches to this Driver */ BOOL WINAPI
pgsql-odbc by date: