Thread: Compiler warnings in psqloodbc 08.03.0200

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

info.c: In function 'PGAPI_GetInfo':
info.c:827: warning: label 'cleanup' defined but not used
info.c: In function 'PGAPI_GetTypeInfo':
info.c:972: warning: label 'cleanup' defined but not used
info.c: In function 'PGAPI_Tables':
info.c:1919: warning: the address of 'table_name' will always evaluate as 'true'
info.c: In function 'PGAPI_Statistics':
info.c:2935: warning: unused variable 'relkind'
info.c: In function 'PGAPI_ColumnPrivileges':
info.c:3487: warning: label 'cleanup' defined but not used
bind.c: In function 'PGAPI_NumParams':
bind.c:449: warning: unused variable 'dollar_quote'
bind.c:449: warning: unused variable 'identifier_quote'
bind.c:449: warning: unused variable 'literal_quote'
bind.c: In function 'CountParameters':
bind.c:626: warning: unused variable 'func'
connection.c: In function 'CC_Constructor':
connection.c:360: warning: implicit declaration of function 'isMsAccess'
connection.c: In function 'CC_create_errormsg':
connection.c:2091: warning: the address of 'msg' will always evaluate as 'true'
connection.c: In function 'LIBPQ_connect':
connection.c:3704: warning: unused variable 'on'
connection.c: In function 'CurrCat':
connection.c:3781: warning: implicit declaration of function 'isMsQuery'
connection.c: At top level:
connection.c:256: warning: 'CC_globals_init' defined but not used
convert.c: In function 'copy_statement_with_parameters':
convert.c:2512: warning: the address of 'curname' will always evaluate as 'true'
convert.c: In function 'ResolveNumericParam':
convert.c:3236: warning: '0' flag ignored with precision and '%d' printf format
convert.c: In function 'prep_params':
convert.c:2302: warning: 'srvquery' may be used uninitialized in this function
convert.c:2302: warning: 'orgquery' may be used uninitialized in this function
convert.c: In function 'convert_escape':
convert.c:4328: warning: 'param_consumed' may be used uninitialized in this function
drvconn.c: In function 'dconn_get_connect_attributes':
drvconn.c:492: warning: passing argument 1 of 'dconn_get_attributes' from incompatible pointer type
drvconn.c: In function 'dconn_get_common_attributes':
drvconn.c:498: warning: passing argument 1 of 'dconn_get_attributes' from incompatible pointer type
drvconn.c: In function 'dconn_get_attributes':
drvconn.c:430: warning: 'last' may be used uninitialized in this function
environ.c: In function 'EN_Constructor':
environ.c:534: warning: label 'cleanup' defined but not used
environ.c:509: warning: unused variable 'func'
execute.c: In function 'PGAPI_Execute':
execute.c:971: warning: implicit declaration of function 'isSqlServr'
options.c: In function 'PGAPI_SetConnectOption':
options.c:503: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c:509: warning: cast to pointer from integer of different size
options.c: In function 'PGAPI_GetConnectOption':
options.c:581: warning: cast from pointer to integer of different size
pgtypes.c: In function 'getCharColumnSize':
pgtypes.c:721: warning: implicit declaration of function 'isSqlServr'
pgtypes.c: At top level:
pgtypes.c:808: warning: 'getTimestampMaxDecimalDigits' defined but not used
psqlodbc.c: In function 'getMutexAttr':
psqlodbc.c:55: warning: implicit declaration of function 'pthread_mutexattr_settype'
psqlodbc.c: At top level:
psqlodbc.c:96: warning: 'finalize_global_cs' defined but not used
qresult.c: In function 'QR_next_tuple':
qresult.c:869: warning: format '%lu' expects type 'long unsigned int *', but argument 3 has type 'SQLUINTEGER *'
qresult.c:920: warning: format '%lu' expects type 'long unsigned int *', but argument 3 has type 'SQLUINTEGER *'
results.c: At top level:
results.c:2093: warning: 'tupleIsAdding' defined but not used
results.c:2113: warning: 'tupleIsUpdating' defined but not used
results.c:2137: warning: 'tupleIsDeleting' defined but not used
results.c:2719: warning: 'IndexExists' defined but not used
socket.c:186: warning: initialization from incompatible pointer type
socket.c: In function 'format_sockerr':
socket.c:206: warning: cast to pointer from integer of different size
socket.c: In function 'SOCK_wait_for_ready':
socket.c:468: warning: 'no_timeout' may be used uninitialized in this function
parse.c: In function 'CheckHasOids':
parse.c:392: warning: too few arguments for format
parse.c:393: warning: the address of 'query' will always evaluate as 'true'
parse.c:413: warning: the address of 'query' will always evaluate as 'true'
parse.c: At top level:
parse.c:548: warning: return type defaults to 'int'
parse.c: In function 'SC_set_SS_columnkey':
parse.c:993: warning: passing argument 2 of 'PGAPI_AllocStmt' from incompatible pointer type
parse.c: In function 'parse_the_statement':
parse.c:1178: warning: cast to pointer from integer of different size
parse.c:1313: warning: the address of 'token' will always evaluate as 'true'
parse.c:1438: warning: the address of 'token' will always evaluate as 'true'
parse.c:1468: warning: the address of 'token' will always evaluate as 'true'
parse.c:1485: warning: the address of 'token' will always evaluate as 'true'
parse.c:1610: warning: the address of 'token' will always evaluate as 'true'
parse.c:1687: warning: the address of 'token' will always evaluate as 'true'
parse.c:1710: warning: the address of 'token' will always evaluate as 'true'
parse.c:1135: warning: 'allocated_size' may be used uninitialized in this function
parse.c:1142: warning: 'column_has_alias' may be used uninitialized in this function
parse.c:1139: warning: 'parse' may be used uninitialized in this function
statement.c: In function 'SendParseRequest':
statement.c:2490: warning: 'end_pidx' may be used uninitialized in this function
statement.c:2490: warning: 'sta_pidx' may be used uninitialized in this function
dlg_specific.c: In function 'getDSNinfo':
dlg_specific.c:896: warning: pointer type mismatch in conditional expression
dlg_specific.c: In function 'writeDriverCommoninfo':
dlg_specific.c:915: warning: comparison with string literal results in unspecified behavior
multibyte.c: In function 'pg_CS_code':
multibyte.c:85: warning: unused variable 'len'
multibyte.c: In function 'get_environment_encoding':
multibyte.c:515: warning: unused variable 'acp'
descriptor.c: In function 'TI_Constructor':
descriptor.c:36: warning: the address of 'qual' will always evaluate as 'true'
descriptor.c: In function 'DC_create_errorinfo':
descriptor.c:587: warning: unused variable 'func'
pgapi30.c: In function 'PGAPI_GetConnectAttr':
pgapi30.c:401: warning: unused variable 'func'
pgapi30.c: In function 'PGAPI_SetConnectAttr':
pgapi30.c:1667: warning: cast from pointer to integer of different size
pgapi30.c: In function 'PGAPI_SetStmtAttr':
pgapi30.c:1870: warning: cast from pointer to integer of different size
pgapi30.c:1873: warning: cast from pointer to integer of different size
mylog.c: In function 'forcelog':
mylog.c:253: warning: format '%u' expects type 'unsigned int', but argument 3 has type 'pthread_t'

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

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/

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

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

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/

Zoltan Boszormenyi wrote:
> 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.

Thanks.

> In pgapi30.c, two instances of
> "cast from pointer to integer of different size"

They may come from the strange handling of unixODBC's
64bit ODBC. Honestly I don't understand how to use
64-bit unixODBC correctly. Probably you can remove the
  warnings by #defining BUILD_REAL_64_BIT_MODE somewhere.

> In psqlodbc.c()::finalize_global_cs() is only used inside
> "#ifdef WIN32" but was defined outside causing a
> "defined but not used" warning.

It is also used in _fini() when __GNUC__ isn't defined.
Though I'm not familiar with *nix systems, it seems
strange to me that there's no function with
__attribute__((destructor)) while init() function
with __attribute__((constrcutor)) is used under
  __GNUC__ mode.

Hiroshi Inoue írta:
> Zoltan Boszormenyi wrote:
>> 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.
>
> Thanks.
>
>> In pgapi30.c, two instances of
>> "cast from pointer to integer of different size"
>
> They may come from the strange handling of unixODBC's
> 64bit ODBC. Honestly I don't understand how to use
> 64-bit unixODBC correctly. Probably you can remove the
>  warnings by #defining BUILD_REAL_64_BIT_MODE somewhere.

I'll try it but the CAST_PTR() seems to be working
in both 32 and 64-bit. BUILD_REAL_64_BIT_MODE
should be defined by the autoconf machinery if needed.

>> In psqlodbc.c()::finalize_global_cs() is only used inside
>> "#ifdef WIN32" but was defined outside causing a
>> "defined but not used" warning.
>
> It is also used in _fini() when __GNUC__ isn't defined.
> Though I'm not familiar with *nix systems, it seems
> strange to me that there's no function with
> __attribute__((destructor)) while init() function
> with __attribute__((constrcutor)) is used under
>  __GNUC__ mode.

So, because of boolean logic:
A v (!A ^ !B) = A v !B
something like below would work:

#if defined(WIN32) || !defined(__GNUC__)
...
#endif

around finalize_global_cs().

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/

Hi.

Information, one reality.

> Zoltan Boszormenyi wrote:

>> In pgapi30.c, two instances of
>> "cast from pointer to integer of different size"
>
> They may come from the strange handling of unixODBC's
> 64bit ODBC. Honestly I don't understand how to use
> 64-bit unixODBC correctly. Probably you can remove the
>  warnings by #defining BUILD_REAL_64_BIT_MODE somewhere.

We tried it by RedHat x86_64. with unixODBC(rpm Package) before psqlODBC release(08.03.0300)
[hiroshi@info psqlodbc-08.03.0300x]$ odbc_config --cflags
-DHAVE_UNISTD_H -DHAVE_PWD_H -DHAVE_SYS_TYPES_H -DHAVE_LONG_LONG -DSIZEOF_LONG=8

<case 1>
This had a big problem in operation.
Driver: BUILD_REAL_64_BIT_MODE
Application: None

<case 2>
This is comfortably.
Driver: BUILD_REAL_64_BIT_MODE
Application: BUILD_REAL_64_BIT_MODE

<case 3>
This is comfortably. (test of a simple query)
Driver: None
Application: BUILD_REAL_64_BIT_MODE

<case 4>
This is comfortably.
Driver: None
Application: None

Therefore, using BUILD_REAL_64_BIT_MODE complicates a problem now....
Note: They are not many tests.

Regards,
Hiroshi Saito

Zoltan Boszormenyi wrote:
> Hiroshi Inoue írta:
>> Zoltan Boszormenyi wrote:
>>> 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.

I forgot to mention that I don't think it's very nice
to remove all the warnings.

>> Thanks.
>>
>>> In pgapi30.c, two instances of
>>> "cast from pointer to integer of different size"
>> They may come from the strange handling of unixODBC's
>> 64bit ODBC. Honestly I don't understand how to use
>> 64-bit unixODBC correctly. Probably you can remove the
>>  warnings by #defining BUILD_REAL_64_BIT_MODE somewhere.
>
> I'll try it but the CAST_PTR() seems to be working
> in both 32 and 64-bit.

Because I've implemeted the driver with the assmption
sizeof(SQLLEN) == sizeof(POINTER), I don't think the
warnings should be removed in such a way.

 > BUILD_REAL_64_BIT_MODE
> should be defined by the autoconf machinery if needed.

As I already mentioned, I don't understand 64-bit unixODBC.
Maybe you have to use 64-bit unixODBC carefully.

>>> In psqlodbc.c()::finalize_global_cs() is only used inside
>>> "#ifdef WIN32" but was defined outside causing a
>>> "defined but not used" warning.
>> It is also used in _fini() when __GNUC__ isn't defined.
>> Though I'm not familiar with *nix systems, it seems
>> strange to me that there's no function with
>> __attribute__((destructor)) while init() function
>> with __attribute__((constrcutor)) is used under
>>  __GNUC__ mode.
>
> So, because of boolean logic:
> A v (!A ^ !B) = A v !B
> something like below would work:
>
> #if defined(WIN32) || !defined(__GNUC__)
> ...
> #endif
>
> around finalize_global_cs().

IMHO initialize and finalize functions should be
  cosidered as a pair and it seems appropriate to issue
warnings when the corresponding finalize function is
not used whereas the initialize function is used.

Hiroshi Inoue írta:
> Zoltan Boszormenyi wrote:
>> Hiroshi Inoue írta:
>>> Zoltan Boszormenyi wrote:
>>>> 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.
>
> I forgot to mention that I don't think it's very nice
> to remove all the warnings.
>
>>> Thanks.
>>>
>>>> In pgapi30.c, two instances of
>>>> "cast from pointer to integer of different size"
>>> They may come from the strange handling of unixODBC's
>>> 64bit ODBC. Honestly I don't understand how to use
>>> 64-bit unixODBC correctly. Probably you can remove the
>>>  warnings by #defining BUILD_REAL_64_BIT_MODE somewhere.
>>
>> I'll try it but the CAST_PTR() seems to be working
>> in both 32 and 64-bit.
>
> Because I've implemeted the driver with the assmption
> sizeof(SQLLEN) == sizeof(POINTER), I don't think the
> warnings should be removed in such a way.
>
> > BUILD_REAL_64_BIT_MODE
>> should be defined by the autoconf machinery if needed.
>
> As I already mentioned, I don't understand 64-bit unixODBC.
> Maybe you have to use 64-bit unixODBC carefully.
>
>>>> In psqlodbc.c()::finalize_global_cs() is only used inside
>>>> "#ifdef WIN32" but was defined outside causing a
>>>> "defined but not used" warning.
>>> It is also used in _fini() when __GNUC__ isn't defined.
>>> Though I'm not familiar with *nix systems, it seems
>>> strange to me that there's no function with
>>> __attribute__((destructor)) while init() function
>>> with __attribute__((constrcutor)) is used under
>>>  __GNUC__ mode.
>>
>> So, because of boolean logic:
>> A v (!A ^ !B) = A v !B
>> something like below would work:
>>
>> #if defined(WIN32) || !defined(__GNUC__)
>> ...
>> #endif
>>
>> around finalize_global_cs().
>
> IMHO initialize and finalize functions should be
>  cosidered as a pair and it seems appropriate to issue
> warnings when the corresponding finalize function is
> not used whereas the initialize function is used.

You can register your finalize function with atexit()
to cleanup when the app finishes.

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/

On Wed, Oct 1, 2008 at 9:40 AM, Hiroshi Saito <z-saito@guitar.ocn.ne.jp> wrote:
>> Zoltan Boszormenyi wrote:
>> They may come from the strange handling of unixODBC's
>> 64bit ODBC. Honestly I don't understand how to use
>> 64-bit unixODBC correctly. Probably you can remove the
>>  warnings by #defining BUILD_REAL_64_BIT_MODE somewhere.


BUILD_REAL_64_BIT_MODE must be defined on 64-bit architectures.
Compiling under 64-bit without this will creates messed up ABI driver
that is NOT ODBC64. If you look on msdn, ODBC64 definitions are
correct only if BUILD_REAL_64_BIT_MODE is defined.

In this mode, SQLLEN is 64-bit. SQLINTEGER is 32-bit.

If the BUILD_REAL_64_BIT_MODE is not defined, then SQLLEN is defined
to be SQLINTEGER which is incorrect for 64-bit ODBC.

Therefore, the only correct scenario is for PostgreSQL's ODBC driver
to work with unixODBC with the BUILD_REAL_64_BIT_MODE for 64-bit and
ignore the other stuff as it is incorrect.

- Adam

PS. On Debian, unixODBC's sqltypes.h has been modified by the
maintainer to include,

/*
 * Failing to define this is *absolutely* broken on 64-bit archs, and we
 * are setting it in the Debian build, so use of this ABI is mandatory.
 * If you don't like it, go build your own non-64-bit-clean library instead.
 * SRL 2006-03-04
 */
#ifndef BUILD_REAL_64_BIT_MODE
#define BUILD_REAL_64_BIT_MODE
#endif

I would just add a check that verifies that when sizeof(long) == 8,
that sizeof(SQLLEN) ==8 as well, otherwise the unixODBC install is not
according to ODBC64 specs.

Hi.

Umm, We can't know in which mode the package (rpm) of unixODBC is built and released.
Supposing it defines as sqltypes.h of Debian compulsorily, it will be used also for our
psqlODBC.
Then, that to which Debian is then supplied is clear. However, unixODBC in which a
definition
is not found does not understand whether it is the 64-bit mode.

I think that the supplier of unixODBC should have the responsibility. it will be clear if
expressed
by "odbc_config --cflags". Then, Is BUILD_REAL_64_BIT_MODE of a definition in some
document? and, How does a user use?

Therefore, I think that supply of the present psqlODBC is the best.

Regards,
Hiroshi Saito

----- Original Message -----
From: "Adam M" <gnuman1@gmail.com>


> On Wed, Oct 1, 2008 at 9:40 AM, Hiroshi Saito <z-saito@guitar.ocn.ne.jp> wrote:
>>> Zoltan Boszormenyi wrote:
>>> They may come from the strange handling of unixODBC's
>>> 64bit ODBC. Honestly I don't understand how to use
>>> 64-bit unixODBC correctly. Probably you can remove the
>>>  warnings by #defining BUILD_REAL_64_BIT_MODE somewhere.
>
>
> BUILD_REAL_64_BIT_MODE must be defined on 64-bit architectures.
> Compiling under 64-bit without this will creates messed up ABI driver
> that is NOT ODBC64. If you look on msdn, ODBC64 definitions are
> correct only if BUILD_REAL_64_BIT_MODE is defined.
>
> In this mode, SQLLEN is 64-bit. SQLINTEGER is 32-bit.
>
> If the BUILD_REAL_64_BIT_MODE is not defined, then SQLLEN is defined
> to be SQLINTEGER which is incorrect for 64-bit ODBC.
>
> Therefore, the only correct scenario is for PostgreSQL's ODBC driver
> to work with unixODBC with the BUILD_REAL_64_BIT_MODE for 64-bit and
> ignore the other stuff as it is incorrect.
>
> - Adam
>
> PS. On Debian, unixODBC's sqltypes.h has been modified by the
> maintainer to include,
>
> /*
> * Failing to define this is *absolutely* broken on 64-bit archs, and we
> * are setting it in the Debian build, so use of this ABI is mandatory.
> * If you don't like it, go build your own non-64-bit-clean library instead.
> * SRL 2006-03-04
> */
> #ifndef BUILD_REAL_64_BIT_MODE
> #define BUILD_REAL_64_BIT_MODE
> #endif
>
> I would just add a check that verifies that when sizeof(long) == 8,
> that sizeof(SQLLEN) ==8 as well, otherwise the unixODBC install is not
> according to ODBC64 specs.
>
> --
> Sent via pgsql-odbc mailing list (pgsql-odbc@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-odbc

On Fri, Oct 3, 2008 at 9:42 AM, Hiroshi Saito <z-saito@guitar.ocn.ne.jp> wrote:
> I think that the supplier of unixODBC should have the responsibility. it
> will be clear if expressed
> by "odbc_config --cflags". Then, Is BUILD_REAL_64_BIT_MODE of a definition
> in some
> document? and, How does a user use?
>
> Therefore, I think that supply of the present psqlODBC is the best.

Hiroshi, sorry about the noise - should have sent this to the mailing
list (gmail interface problems :)

While you are correct that unixODBC should be where this is fixed, and
I'll contact the maintainer, psqlODBC should check if a sane ODBC
environment is provided..

What I'm trying to say, is that ODBC, like an application, can be
compiled as a 32-bit application or a 64-bit application. On AMD64

As a 32-bit application, we have sizeof(long) == 4
As a 64-bit application, we have sizeof(long) == 8

Similarly, as an ODBC application, it can be compiled under 32-bit
mode, or a 64-bit mode.

A 32-bit ODBC application, we have sizeof(SQLLEN) == 4
A 64-bit ODBC application, we have sizeof(SQLLEN) == 8

This is exactly what Microsoft is using and AFAIK, they are still the
people that control the ODBC specs,
  http://msdn.microsoft.com/en-us/library/ms716287.aspx

Now, unixODBC when compiled as a 64-bit library, can be compiled with
the BUILD_REAL_64_BIT_MODE or without it. Without defining this
constant, the ODBC API supplied by unixODBC is *not* compatible with
64-bit ODBC specs. It is *wrong*.

From the sqltypes.h again, this from from unixODBC developer,

/*
 * I (Nick) have made these changes, to cope with the new 3.52 MS
 * changes for 64 bit ODBC, but looking at MS's spec they havn't
 * finished it themself. For example, SQLBindCol now expects the
 * indicator variable to be a SQLLEN which then is a pointer to
 * a 64 bit value. However the online book that comes with the
 * headers, then goes on to describe the indicator_ptr in the
 * descriptor record (which is set by SQLBindCol) as a pointer
 * to a SQLINTEGER (32 bit). So I don't think its ready for the
 * big time yet. Thats not to mention all the ODBC apps on 64 bit
 * platforms that this would break...
 *
 * I have just discovered that on win64 sizeof(long) == 4, so its
 * all smoke and mirrors...
 *
 */

As you can see, this is not applicable anymore as per the msdn link.
On 64-bit specs SQLLEN is defined to be 64-bit integer, and SQLINTEGER
is defined to be int, which is just 32-bit integer.

I would like to add that testing for broken unixODBC compilations on
64-bit distributions is easy and important at same time. If some
define BUILD_REAL_64_BIT_MODE while others do not, then the
applications that use psqlODBC will not be compatible between
distributions and even not 64-bit clean.


I can add a check to configure.ac to check that sizeof(SQLLEN) is not
sizeof(SQLINTEGER) on 64-bit platforms and post the patch here.

- Adam

PS. I've already been bitten by this bug, kind of indirectly. Qt's
ODBC layer was tested with unixODBC on FreeBSD. They have not defined
BUILD_REAL_64_BIT_MODE so now their ODBC is broken not only on Debian,
but also on Windows 64-bit as they were casting SQLINTEGER into
SQLLEN.. Somehow though, they have not noticed the mistake.
"QODBC plugin code tries to cast a SQLINTEGER pointer to a SQLLEN
pointer" - oops.

Hiroshi Inoue írta:
> Zoltan Boszormenyi wrote:
>> Hiroshi Inoue írta:
>>> Zoltan Boszormenyi wrote:
>>>> 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.
>
> I forgot to mention that I don't think it's very nice
> to remove all the warnings.

Sorry to refresh such an old thread, but I just got a choice
to build psqlODBC on Solaris10/Sparc with sunw cc instead
of gcc. I get tons of warnings like this:

"environ.c", line 325: warning: argument #3 is incompatible with prototype:
        prototype: pointer to const unsigned char : "environ.c", line 113
        argument : pointer to char

The cause is the same as for the "pointers differ in signedness"
under GCC. I can agree with your assertion that "it's not very nice
to remove all warnings" in the sense that using manual casts may
only cover bugs. But fixing the above warning using proper function
prototypes is still desired. A function that expects constant message
strings should declare that parameter as "char *" instead of
"unsigned char *" and it would avoid warnings like the above.

Best regards,
Zoltán Böszörményi

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/