Re: 7.1 odbc bug & patch

Your name : Dave Bodenstab
Your email address : imdave(at)mcs(dot)net

System Configuration
---------------------
Architecture (example: Intel Pentium) : (shouldn't matter) Intel x86

Operating System (example: Linux 2.0.26 ELF) : (shouldn't matter) FreeBSD

PostgreSQL version (example: PostgreSQL-7.1): PostgreSQL-7.1

Compiler used (example: gcc 2.8.0) : (shouldn't matter) gcc 2.7.2.1

Please enter a FULL description of your problem:
------------------------------------------------
I installed postgres 7.1 with --enable-odbc. I then installed
tclodbc (http://sourceforge.net/projects/tclodbc) and libiodbc-2.50.3
(http://www.iodbc.org/dist/libiodbc-2.50.3.tar.gz) I could not
get either to work... postgres would not find the global odbcinst.ini
file. I traced this to src/interfaces/odbc/gpps.c -- here are the
many things I think are wrong:

40 GetPrivateProfileString(char *theSection, /* section name */
:
50 {
51 char buf[MAXPGPATH];
52 char *ptr = 0;
:
63 int j = 0;

64 j = strlen(theIniFileName) + 1;
65 ptr = (char *) getpwuid(getuid()); /* get user info */

66 if (ptr == NULL)
67 {
68 if (MAXPGPATH - 1 < j)
69 theIniFileName[MAXPGPATH - 1] = '\0';

70 sprintf(buf, "%s", theIniFileName);
71 }
72 ptr = ((struct passwd *) ptr)->pw_dir; /* get user home dir */
73 if (ptr == NULL || *ptr == '\0')
74 ptr = "/home";

75 /*
76 * This doesn't make it so we find an ini file but allows normal
77 * processing to continue further on down. The likelihood is that the
78 * file won't be found and thus the default value will be returned.
79 */
80 if (MAXPGPATH - 1 < strlen(ptr) + j)
81 {
82 if (MAXPGPATH - 1 < strlen(ptr))
83 ptr[MAXPGPATH - 1] = '\0';
84 else
85 theIniFileName[MAXPGPATH - 1 - strlen(ptr)] = '\0';
86 }

87 sprintf(buf, "%s/%s", ptr, theIniFileName);

88 /*
89 * This code makes it so that a file in the users home dir overrides a
90 * the "default" file as passed in
91 */
92 aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
93 if (!aFile)
94 {
95 sprintf(buf, "%s", theIniFileName);
96 aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
97 }

+ Line 65 sets ptr, line 66 checks for NULL but line 72 dereferences
ptr anyway. Then line 73 checks for NULL again!?
+ Line 87 unconditionally ignores what line 70 did.
+ Lines 69, 83 and 85 modify the value passed to the function unbeknownst
to the calling function. The values could be a static string or might
be further used by the calling function.
+ Truncating the strings might result in a string that accidently matches
another, totally non-related file.
+ Line 96 checks ``buf?'', but buf[] is an array and the test will *always*
be true.
+ The globally installed odbcinst.ini file is never checked. This is
why I couldn't get things to work -- it was looking for odbcinst.ini
only in my home directory or the current directory.

Please describe a way to repeat the problem. Please try to provide a
concise reproducible example, if at all possible:
----------------------------------------------------------------------
Run tclodbc and do a ``database db <DSNname>'' where ``DSNname'' is
one of the DSN's in /usr/local/etc/odbcinst.ini (or wherever the
global ini file is installed.) The result is always the error
message that ``one of server,port,database,etc. are missing''.

Run libiodbc-2.50.3/samples/odbctest <DSNname>. The command fails
to connect to the database and just exits.

If you know how this problem might be fixed, list the solution below:
---------------------------------------------------------------------
I couldn't find any documentation as to what the correct behavior should
be, and the comments aren't any clearer. I changed the code to look for
the odbcinst.ini file in 1) $HOME, 2) the current directory and 3) the
globally installed file. I also fixed all the problems I mentioned
above. Here is the patch, it works for me. Use it any way you want.

--- gpps.c 2001/06/06 02:53:05 7.1
+++ gpps.c 2001/06/06 17:23:39 7.1.1.3
@@ -35,6 +35,7 @@
#include <string.h>
#include "misc.h"
#include "gpps.h"
+#include "dlg_specific.h"

#ifndef TRUE
#define TRUE ((BOOL)1)
@@ -44,6 +45,12 @@
#endif


+/*
+ * theIniFileName is searched for in:
+ * $HOME/theIniFileName
+ * theIniFileName
+ * ODBCINST_INI
+ */
DWORD
GetPrivateProfileString(char *theSection, /* section name */
char *theKey, /* search key name */
@@ -68,48 +75,36 @@
size_t aReturnLength = 0;
BOOL aSectionFound = FALSE;
BOOL aKeyFound = FALSE;
- int j = 0;

- j = strlen(theIniFileName) + 1;
ptr = (char *) getpwuid(getuid()); /* get user info */

- if (ptr == NULL)
- {
- if (MAXPGPATH - 1 < j)
- theIniFileName[MAXPGPATH - 1] = '\0';
-
- sprintf(buf, "%s", theIniFileName);
- }
- ptr = ((struct passwd *) ptr)->pw_dir; /* get user home dir */
- if (ptr == NULL || *ptr == '\0')
+ if (ptr == NULL || (((struct passwd *) ptr)->pw_dir) == NULL || *(((struct passwd *) ptr)->pw_dir) == '\0')
ptr = "/home";
+ else
+ ptr = ((struct passwd *) ptr)->pw_dir; /* get user home dir */

/*
- * This doesn't make it so we find an ini file but allows normal
- * processing to continue further on down. The likelihood is that the
- * file won't be found and thus the default value will be returned.
+ * If it can't be opened because the paths are too long, then
+ * skip it, don't just truncate the path string... The truncated path
+ * might accidently be an existing file. The default value will be
+ * returned instead.
*/
- if (MAXPGPATH - 1 < strlen(ptr) + j)
+ if (MAXPGPATH - 1 >= strlen(ptr) + 1 + strlen(theIniFileName))
{
- if (MAXPGPATH - 1 < strlen(ptr))
- ptr[MAXPGPATH - 1] = '\0';
- else
- theIniFileName[MAXPGPATH - 1 - strlen(ptr)] = '\0';
+ sprintf(buf, "%s/%s", ptr, theIniFileName);
+ aFile = (FILE *) fopen(buf, PG_BINARY_R);
}

- sprintf(buf, "%s/%s", ptr, theIniFileName);
-
/*
* This code makes it so that a file in the users home dir overrides a
* the "default" file as passed in
*/
- aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
if (!aFile)
{
- sprintf(buf, "%s", theIniFileName);
- aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
+ aFile = (FILE *) fopen(theIniFileName, PG_BINARY_R);
+ if (!aFile)
+ aFile = (FILE *) fopen(ODBCINST_INI, PG_BINARY_R);
}
-

aLength = (theDefault == NULL) ? 0 : strlen(theDefault);

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

>
> Your name : Dave Bodenstab
> Your email address : imdave(at)mcs(dot)net
>
>
> System Configuration
> ---------------------
> Architecture (example: Intel Pentium) : (shouldn't matter) Intel x86
>
> Operating System (example: Linux 2.0.26 ELF) : (shouldn't matter) FreeBSD
>
> PostgreSQL version (example: PostgreSQL-7.1): PostgreSQL-7.1
>
> Compiler used (example: gcc 2.8.0) : (shouldn't matter) gcc 2.7.2.1
>
>
>
> Please enter a FULL description of your problem:
> ------------------------------------------------
> I installed postgres 7.1 with --enable-odbc. I then installed
> tclodbc (http://sourceforge.net/projects/tclodbc) and libiodbc-2.50.3
> (http://www.iodbc.org/dist/libiodbc-2.50.3.tar.gz) I could not
> get either to work... postgres would not find the global odbcinst.ini
> file. I traced this to src/interfaces/odbc/gpps.c -- here are the
> many things I think are wrong:
>
> 40 GetPrivateProfileString(char *theSection, /* section name */
> :
> 50 {
> 51 char buf[MAXPGPATH];
> 52 char *ptr = 0;
> :
> 63 int j = 0;
>
> 64 j = strlen(theIniFileName) + 1;
> 65 ptr = (char *) getpwuid(getuid()); /* get user info */
>
> 66 if (ptr == NULL)
> 67 {
> 68 if (MAXPGPATH - 1 < j)
> 69 theIniFileName[MAXPGPATH - 1] = '\0';
>
> 70 sprintf(buf, "%s", theIniFileName);
> 71 }
> 72 ptr = ((struct passwd *) ptr)->pw_dir; /* get user home dir */
> 73 if (ptr == NULL || *ptr == '\0')
> 74 ptr = "/home";
>
> 75 /*
> 76 * This doesn't make it so we find an ini file but allows normal
> 77 * processing to continue further on down. The likelihood is that the
> 78 * file won't be found and thus the default value will be returned.
> 79 */
> 80 if (MAXPGPATH - 1 < strlen(ptr) + j)
> 81 {
> 82 if (MAXPGPATH - 1 < strlen(ptr))
> 83 ptr[MAXPGPATH - 1] = '\0';
> 84 else
> 85 theIniFileName[MAXPGPATH - 1 - strlen(ptr)] = '\0';
> 86 }
>
> 87 sprintf(buf, "%s/%s", ptr, theIniFileName);
>
> 88 /*
> 89 * This code makes it so that a file in the users home dir overrides a
> 90 * the "default" file as passed in
> 91 */
> 92 aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
> 93 if (!aFile)
> 94 {
> 95 sprintf(buf, "%s", theIniFileName);
> 96 aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
> 97 }
>
> + Line 65 sets ptr, line 66 checks for NULL but line 72 dereferences
> ptr anyway. Then line 73 checks for NULL again!?
> + Line 87 unconditionally ignores what line 70 did.
> + Lines 69, 83 and 85 modify the value passed to the function unbeknownst
> to the calling function. The values could be a static string or might
> be further used by the calling function.
> + Truncating the strings might result in a string that accidently matches
> another, totally non-related file.
> + Line 96 checks ``buf?'', but buf[] is an array and the test will *always*
> be true.
> + The globally installed odbcinst.ini file is never checked. This is
> why I couldn't get things to work -- it was looking for odbcinst.ini
> only in my home directory or the current directory.
>
>
>
> Please describe a way to repeat the problem. Please try to provide a
> concise reproducible example, if at all possible:
> ----------------------------------------------------------------------
> Run tclodbc and do a ``database db <DSNname>'' where ``DSNname'' is
> one of the DSN's in /usr/local/etc/odbcinst.ini (or wherever the
> global ini file is installed.) The result is always the error
> message that ``one of server,port,database,etc. are missing''.
>
> Run libiodbc-2.50.3/samples/odbctest <DSNname>. The command fails
> to connect to the database and just exits.
>
>
>
> If you know how this problem might be fixed, list the solution below:
> ---------------------------------------------------------------------
> I couldn't find any documentation as to what the correct behavior should
> be, and the comments aren't any clearer. I changed the code to look for
> the odbcinst.ini file in 1) $HOME, 2) the current directory and 3) the
> globally installed file. I also fixed all the problems I mentioned
> above. Here is the patch, it works for me. Use it any way you want.
>
> --- gpps.c 2001/06/06 02:53:05 7.1
> +++ gpps.c 2001/06/06 17:23:39 7.1.1.3
> @@ -35,6 +35,7 @@
> #include <string.h>
> #include "misc.h"
> #include "gpps.h"
> +#include "dlg_specific.h"
>
> #ifndef TRUE
> #define TRUE ((BOOL)1)
> @@ -44,6 +45,12 @@
> #endif
>
>
> +/*
> + * theIniFileName is searched for in:
> + * $HOME/theIniFileName
> + * theIniFileName
> + * ODBCINST_INI
> + */
> DWORD
> GetPrivateProfileString(char *theSection, /* section name */
> char *theKey, /* search key name */
> @@ -68,48 +75,36 @@
> size_t aReturnLength = 0;
> BOOL aSectionFound = FALSE;
> BOOL aKeyFound = FALSE;
> - int j = 0;
>
> - j = strlen(theIniFileName) + 1;
> ptr = (char *) getpwuid(getuid()); /* get user info */
>
> - if (ptr == NULL)
> - {
> - if (MAXPGPATH - 1 < j)
> - theIniFileName[MAXPGPATH - 1] = '\0';
> -
> - sprintf(buf, "%s", theIniFileName);
> - }
> - ptr = ((struct passwd *) ptr)->pw_dir; /* get user home dir */
> - if (ptr == NULL || *ptr == '\0')
> + if (ptr == NULL || (((struct passwd *) ptr)->pw_dir) == NULL || *(((struct passwd *) ptr)->pw_dir) == '\0')
> ptr = "/home";
> + else
> + ptr = ((struct passwd *) ptr)->pw_dir; /* get user home dir */
>
> /*
> - * This doesn't make it so we find an ini file but allows normal
> - * processing to continue further on down. The likelihood is that the
> - * file won't be found and thus the default value will be returned.
> + * If it can't be opened because the paths are too long, then
> + * skip it, don't just truncate the path string... The truncated path
> + * might accidently be an existing file. The default value will be
> + * returned instead.
> */
> - if (MAXPGPATH - 1 < strlen(ptr) + j)
> + if (MAXPGPATH - 1 >= strlen(ptr) + 1 + strlen(theIniFileName))
> {
> - if (MAXPGPATH - 1 < strlen(ptr))
> - ptr[MAXPGPATH - 1] = '\0';
> - else
> - theIniFileName[MAXPGPATH - 1 - strlen(ptr)] = '\0';
> + sprintf(buf, "%s/%s", ptr, theIniFileName);
> + aFile = (FILE *) fopen(buf, PG_BINARY_R);
> }
>
> - sprintf(buf, "%s/%s", ptr, theIniFileName);
> -
> /*
> * This code makes it so that a file in the users home dir overrides a
> * the "default" file as passed in
> */
> - aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
> if (!aFile)
> {
> - sprintf(buf, "%s", theIniFileName);
> - aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
> + aFile = (FILE *) fopen(theIniFileName, PG_BINARY_R);
> + if (!aFile)
> + aFile = (FILE *) fopen(ODBCINST_INI, PG_BINARY_R);
> }
> -
>
> aLength = (theDefault == NULL) ? 0 : strlen(theDefault);
>
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Wow, if all our code was as bad as this ODBC snipped we be in serious
trouble. :-)

>
> Your name : Dave Bodenstab
> Your email address : imdave(at)mcs(dot)net
>
>
> System Configuration
> ---------------------
> Architecture (example: Intel Pentium) : (shouldn't matter) Intel x86
>
> Operating System (example: Linux 2.0.26 ELF) : (shouldn't matter) FreeBSD
>
> PostgreSQL version (example: PostgreSQL-7.1): PostgreSQL-7.1
>
> Compiler used (example: gcc 2.8.0) : (shouldn't matter) gcc 2.7.2.1
>
>
>
> Please enter a FULL description of your problem:
> ------------------------------------------------
> I installed postgres 7.1 with --enable-odbc. I then installed
> tclodbc (http://sourceforge.net/projects/tclodbc) and libiodbc-2.50.3
> (http://www.iodbc.org/dist/libiodbc-2.50.3.tar.gz) I could not
> get either to work... postgres would not find the global odbcinst.ini
> file. I traced this to src/interfaces/odbc/gpps.c -- here are the
> many things I think are wrong:
>
> 40 GetPrivateProfileString(char *theSection, /* section name */
> :
> 50 {
> 51 char buf[MAXPGPATH];
> 52 char *ptr = 0;
> :
> 63 int j = 0;
>
> 64 j = strlen(theIniFileName) + 1;
> 65 ptr = (char *) getpwuid(getuid()); /* get user info */
>
> 66 if (ptr == NULL)
> 67 {
> 68 if (MAXPGPATH - 1 < j)
> 69 theIniFileName[MAXPGPATH - 1] = '\0';
>
> 70 sprintf(buf, "%s", theIniFileName);
> 71 }
> 72 ptr = ((struct passwd *) ptr)->pw_dir; /* get user home dir */
> 73 if (ptr == NULL || *ptr == '\0')
> 74 ptr = "/home";
>
> 75 /*
> 76 * This doesn't make it so we find an ini file but allows normal
> 77 * processing to continue further on down. The likelihood is that the
> 78 * file won't be found and thus the default value will be returned.
> 79 */
> 80 if (MAXPGPATH - 1 < strlen(ptr) + j)
> 81 {
> 82 if (MAXPGPATH - 1 < strlen(ptr))
> 83 ptr[MAXPGPATH - 1] = '\0';
> 84 else
> 85 theIniFileName[MAXPGPATH - 1 - strlen(ptr)] = '\0';
> 86 }
>
> 87 sprintf(buf, "%s/%s", ptr, theIniFileName);
>
> 88 /*
> 89 * This code makes it so that a file in the users home dir overrides a
> 90 * the "default" file as passed in
> 91 */
> 92 aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
> 93 if (!aFile)
> 94 {
> 95 sprintf(buf, "%s", theIniFileName);
> 96 aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
> 97 }
>
> + Line 65 sets ptr, line 66 checks for NULL but line 72 dereferences
> ptr anyway. Then line 73 checks for NULL again!?
> + Line 87 unconditionally ignores what line 70 did.
> + Lines 69, 83 and 85 modify the value passed to the function unbeknownst
> to the calling function. The values could be a static string or might
> be further used by the calling function.
> + Truncating the strings might result in a string that accidently matches
> another, totally non-related file.
> + Line 96 checks ``buf?'', but buf[] is an array and the test will *always*
> be true.
> + The globally installed odbcinst.ini file is never checked. This is
> why I couldn't get things to work -- it was looking for odbcinst.ini
> only in my home directory or the current directory.
>
>
>
> Please describe a way to repeat the problem. Please try to provide a
> concise reproducible example, if at all possible:
> ----------------------------------------------------------------------
> Run tclodbc and do a ``database db <DSNname>'' where ``DSNname'' is
> one of the DSN's in /usr/local/etc/odbcinst.ini (or wherever the
> global ini file is installed.) The result is always the error
> message that ``one of server,port,database,etc. are missing''.
>
> Run libiodbc-2.50.3/samples/odbctest <DSNname>. The command fails
> to connect to the database and just exits.
>
>
>
> If you know how this problem might be fixed, list the solution below:
> ---------------------------------------------------------------------
> I couldn't find any documentation as to what the correct behavior should
> be, and the comments aren't any clearer. I changed the code to look for
> the odbcinst.ini file in 1) $HOME, 2) the current directory and 3) the
> globally installed file. I also fixed all the problems I mentioned
> above. Here is the patch, it works for me. Use it any way you want.
>
> --- gpps.c 2001/06/06 02:53:05 7.1
> +++ gpps.c 2001/06/06 17:23:39 7.1.1.3
> @@ -35,6 +35,7 @@
> #include <string.h>
> #include "misc.h"
> #include "gpps.h"
> +#include "dlg_specific.h"
>
> #ifndef TRUE
> #define TRUE ((BOOL)1)
> @@ -44,6 +45,12 @@
> #endif
>
>
> +/*
> + * theIniFileName is searched for in:
> + * $HOME/theIniFileName
> + * theIniFileName
> + * ODBCINST_INI
> + */
> DWORD
> GetPrivateProfileString(char *theSection, /* section name */
> char *theKey, /* search key name */
> @@ -68,48 +75,36 @@
> size_t aReturnLength = 0;
> BOOL aSectionFound = FALSE;
> BOOL aKeyFound = FALSE;
> - int j = 0;
>
> - j = strlen(theIniFileName) + 1;
> ptr = (char *) getpwuid(getuid()); /* get user info */
>
> - if (ptr == NULL)
> - {
> - if (MAXPGPATH - 1 < j)
> - theIniFileName[MAXPGPATH - 1] = '\0';
> -
> - sprintf(buf, "%s", theIniFileName);
> - }
> - ptr = ((struct passwd *) ptr)->pw_dir; /* get user home dir */
> - if (ptr == NULL || *ptr == '\0')
> + if (ptr == NULL || (((struct passwd *) ptr)->pw_dir) == NULL || *(((struct passwd *) ptr)->pw_dir) == '\0')
> ptr = "/home";
> + else
> + ptr = ((struct passwd *) ptr)->pw_dir; /* get user home dir */
>
> /*
> - * This doesn't make it so we find an ini file but allows normal
> - * processing to continue further on down. The likelihood is that the
> - * file won't be found and thus the default value will be returned.
> + * If it can't be opened because the paths are too long, then
> + * skip it, don't just truncate the path string... The truncated path
> + * might accidently be an existing file. The default value will be
> + * returned instead.
> */
> - if (MAXPGPATH - 1 < strlen(ptr) + j)
> + if (MAXPGPATH - 1 >= strlen(ptr) + 1 + strlen(theIniFileName))
> {
> - if (MAXPGPATH - 1 < strlen(ptr))
> - ptr[MAXPGPATH - 1] = '\0';
> - else
> - theIniFileName[MAXPGPATH - 1 - strlen(ptr)] = '\0';
> + sprintf(buf, "%s/%s", ptr, theIniFileName);
> + aFile = (FILE *) fopen(buf, PG_BINARY_R);
> }
>
> - sprintf(buf, "%s/%s", ptr, theIniFileName);
> -
> /*
> * This code makes it so that a file in the users home dir overrides a
> * the "default" file as passed in
> */
> - aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
> if (!aFile)
> {
> - sprintf(buf, "%s", theIniFileName);
> - aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
> + aFile = (FILE *) fopen(theIniFileName, PG_BINARY_R);
> + if (!aFile)
> + aFile = (FILE *) fopen(ODBCINST_INI, PG_BINARY_R);
> }
> -
>
> aLength = (theDefault == NULL) ? 0 : strlen(theDefault);
>
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

> -----Original Message-----
> From: Bruce Momjian
>
> Wow, if all our code was as bad as this ODBC snipped we be in serious
> trouble. :-)
>

Probably gpps.c has caused ODBC troubles under unix(
gpps.c isn't needed at all under Windows). I'm happy if
someone maintains it and would reply to the inquiries
about ODBC under unix.

[snip]

> >
> > Please enter a FULL description of your problem:
> > ------------------------------------------------
> > I installed postgres 7.1 with --enable-odbc. I then installed
> > tclodbc (http://sourceforge.net/projects/tclodbc) and libiodbc-2.50.3
> > (http://www.iodbc.org/dist/libiodbc-2.50.3.tar.gz) I could not
> > get either to work... postgres would not find the global odbcinst.ini
> > file.

AFAIK iODBC doesn't need odbcinst.ini though I don't know
about tclodbc. In addtion psqlodbc searches odbcinst.ini
in a directory.

regards,
Hiroshi Inoue

> > -----Original Message-----
> > From: Bruce Momjian
> >
> > Wow, if all our code was as bad as this ODBC snipped we be in serious
> > trouble. :-)
> >
>
> Probably gpps.c has caused ODBC troubles under unix(
> gpps.c isn't needed at all under Windows). I'm happy if
> someone maintains it and would reply to the inquiries
> about ODBC under unix.

Thanks for the info. I will apply the patch now.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Thanks. Patch applied.

>
> Your name : Dave Bodenstab
> Your email address : imdave(at)mcs(dot)net
>
>
> System Configuration
> ---------------------
> Architecture (example: Intel Pentium) : (shouldn't matter) Intel x86
>
> Operating System (example: Linux 2.0.26 ELF) : (shouldn't matter) FreeBSD
>
> PostgreSQL version (example: PostgreSQL-7.1): PostgreSQL-7.1
>
> Compiler used (example: gcc 2.8.0) : (shouldn't matter) gcc 2.7.2.1
>
>
>
> Please enter a FULL description of your problem:
> ------------------------------------------------
> I installed postgres 7.1 with --enable-odbc. I then installed
> tclodbc (http://sourceforge.net/projects/tclodbc) and libiodbc-2.50.3
> (http://www.iodbc.org/dist/libiodbc-2.50.3.tar.gz) I could not
> get either to work... postgres would not find the global odbcinst.ini
> file. I traced this to src/interfaces/odbc/gpps.c -- here are the
> many things I think are wrong:
>
> 40 GetPrivateProfileString(char *theSection, /* section name */
> :
> 50 {
> 51 char buf[MAXPGPATH];
> 52 char *ptr = 0;
> :
> 63 int j = 0;
>
> 64 j = strlen(theIniFileName) + 1;
> 65 ptr = (char *) getpwuid(getuid()); /* get user info */
>
> 66 if (ptr == NULL)
> 67 {
> 68 if (MAXPGPATH - 1 < j)
> 69 theIniFileName[MAXPGPATH - 1] = '\0';
>
> 70 sprintf(buf, "%s", theIniFileName);
> 71 }
> 72 ptr = ((struct passwd *) ptr)->pw_dir; /* get user home dir */
> 73 if (ptr == NULL || *ptr == '\0')
> 74 ptr = "/home";
>
> 75 /*
> 76 * This doesn't make it so we find an ini file but allows normal
> 77 * processing to continue further on down. The likelihood is that the
> 78 * file won't be found and thus the default value will be returned.
> 79 */
> 80 if (MAXPGPATH - 1 < strlen(ptr) + j)
> 81 {
> 82 if (MAXPGPATH - 1 < strlen(ptr))
> 83 ptr[MAXPGPATH - 1] = '\0';
> 84 else
> 85 theIniFileName[MAXPGPATH - 1 - strlen(ptr)] = '\0';
> 86 }
>
> 87 sprintf(buf, "%s/%s", ptr, theIniFileName);
>
> 88 /*
> 89 * This code makes it so that a file in the users home dir overrides a
> 90 * the "default" file as passed in
> 91 */
> 92 aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
> 93 if (!aFile)
> 94 {
> 95 sprintf(buf, "%s", theIniFileName);
> 96 aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
> 97 }
>
> + Line 65 sets ptr, line 66 checks for NULL but line 72 dereferences
> ptr anyway. Then line 73 checks for NULL again!?
> + Line 87 unconditionally ignores what line 70 did.
> + Lines 69, 83 and 85 modify the value passed to the function unbeknownst
> to the calling function. The values could be a static string or might
> be further used by the calling function.
> + Truncating the strings might result in a string that accidently matches
> another, totally non-related file.
> + Line 96 checks ``buf?'', but buf[] is an array and the test will *always*
> be true.
> + The globally installed odbcinst.ini file is never checked. This is
> why I couldn't get things to work -- it was looking for odbcinst.ini
> only in my home directory or the current directory.
>
>
>
> Please describe a way to repeat the problem. Please try to provide a
> concise reproducible example, if at all possible:
> ----------------------------------------------------------------------
> Run tclodbc and do a ``database db <DSNname>'' where ``DSNname'' is
> one of the DSN's in /usr/local/etc/odbcinst.ini (or wherever the
> global ini file is installed.) The result is always the error
> message that ``one of server,port,database,etc. are missing''.
>
> Run libiodbc-2.50.3/samples/odbctest <DSNname>. The command fails
> to connect to the database and just exits.
>
>
>
> If you know how this problem might be fixed, list the solution below:
> ---------------------------------------------------------------------
> I couldn't find any documentation as to what the correct behavior should
> be, and the comments aren't any clearer. I changed the code to look for
> the odbcinst.ini file in 1) $HOME, 2) the current directory and 3) the
> globally installed file. I also fixed all the problems I mentioned
> above. Here is the patch, it works for me. Use it any way you want.
>
> --- gpps.c 2001/06/06 02:53:05 7.1
> +++ gpps.c 2001/06/06 17:23:39 7.1.1.3
> @@ -35,6 +35,7 @@
> #include <string.h>
> #include "misc.h"
> #include "gpps.h"
> +#include "dlg_specific.h"
>
> #ifndef TRUE
> #define TRUE ((BOOL)1)
> @@ -44,6 +45,12 @@
> #endif
>
>
> +/*
> + * theIniFileName is searched for in:
> + * $HOME/theIniFileName
> + * theIniFileName
> + * ODBCINST_INI
> + */
> DWORD
> GetPrivateProfileString(char *theSection, /* section name */
> char *theKey, /* search key name */
> @@ -68,48 +75,36 @@
> size_t aReturnLength = 0;
> BOOL aSectionFound = FALSE;
> BOOL aKeyFound = FALSE;
> - int j = 0;
>
> - j = strlen(theIniFileName) + 1;
> ptr = (char *) getpwuid(getuid()); /* get user info */
>
> - if (ptr == NULL)
> - {
> - if (MAXPGPATH - 1 < j)
> - theIniFileName[MAXPGPATH - 1] = '\0';
> -
> - sprintf(buf, "%s", theIniFileName);
> - }
> - ptr = ((struct passwd *) ptr)->pw_dir; /* get user home dir */
> - if (ptr == NULL || *ptr == '\0')
> + if (ptr == NULL || (((struct passwd *) ptr)->pw_dir) == NULL || *(((struct passwd *) ptr)->pw_dir) == '\0')
> ptr = "/home";
> + else
> + ptr = ((struct passwd *) ptr)->pw_dir; /* get user home dir */
>
> /*
> - * This doesn't make it so we find an ini file but allows normal
> - * processing to continue further on down. The likelihood is that the
> - * file won't be found and thus the default value will be returned.
> + * If it can't be opened because the paths are too long, then
> + * skip it, don't just truncate the path string... The truncated path
> + * might accidently be an existing file. The default value will be
> + * returned instead.
> */
> - if (MAXPGPATH - 1 < strlen(ptr) + j)
> + if (MAXPGPATH - 1 >= strlen(ptr) + 1 + strlen(theIniFileName))
> {
> - if (MAXPGPATH - 1 < strlen(ptr))
> - ptr[MAXPGPATH - 1] = '\0';
> - else
> - theIniFileName[MAXPGPATH - 1 - strlen(ptr)] = '\0';
> + sprintf(buf, "%s/%s", ptr, theIniFileName);
> + aFile = (FILE *) fopen(buf, PG_BINARY_R);
> }
>
> - sprintf(buf, "%s/%s", ptr, theIniFileName);
> -
> /*
> * This code makes it so that a file in the users home dir overrides a
> * the "default" file as passed in
> */
> - aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
> if (!aFile)
> {
> - sprintf(buf, "%s", theIniFileName);
> - aFile = (FILE *) (buf ? fopen(buf, PG_BINARY_R) : NULL);
> + aFile = (FILE *) fopen(theIniFileName, PG_BINARY_R);
> + if (!aFile)
> + aFile = (FILE *) fopen(ODBCINST_INI, PG_BINARY_R);
> }
> -
>
> aLength = (theDefault == NULL) ? 0 : strlen(theDefault);
>
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Bruce Momjian wrote:
>
> > > -----Original Message-----
> > > From: Bruce Momjian
> > >
> > > Wow, if all our code was as bad as this ODBC snipped we be in serious
> > > trouble. :-)
> > >
> >
> > Probably gpps.c has caused ODBC troubles under unix(
> > gpps.c isn't needed at all under Windows). I'm happy if
> > someone maintains it and would reply to the inquiries
> > about ODBC under unix.
>
> Thanks for the info. I will apply the patch now.
>

Hmm I don't think the patch is proper.
Someone must maintain ODBC stuff under unix.

regards,
Hiroshi Inoue

> Bruce Momjian wrote:
> >
> > > > -----Original Message-----
> > > > From: Bruce Momjian
> > > >
> > > > Wow, if all our code was as bad as this ODBC snipped we be in serious
> > > > trouble. :-)
> > > >
> > >
> > > Probably gpps.c has caused ODBC troubles under unix(
> > > gpps.c isn't needed at all under Windows). I'm happy if
> > > someone maintains it and would reply to the inquiries
> > > about ODBC under unix.
> >
> > Thanks for the info. I will apply the patch now.
> >
>
> Hmm I don't think the patch is proper.
> Someone must maintain ODBC stuff under unix.

Oh, I thought you liked it. Is the new better than the old? I assumed
it was.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Bruce Momjian wrote:
>
> > Bruce Momjian wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Bruce Momjian
> > > > >
> > > > > Wow, if all our code was as bad as this ODBC snipped we be in serious
> > > > > trouble. :-)
> > > > >
> > > >
> > > > Probably gpps.c has caused ODBC troubles under unix(
> > > > gpps.c isn't needed at all under Windows). I'm happy if
> > > > someone maintains it and would reply to the inquiries
> > > > about ODBC under unix.
> > >
> > > Thanks for the info. I will apply the patch now.
> > >
> >
> > Hmm I don't think the patch is proper.
> > Someone must maintain ODBC stuff under unix.
>
> Oh, I thought you liked it. Is the new better than the old?

Partly maybe. But the handling of odbcinst.ini seems
to be different from its original intension.

What I expect is that someone who knows well about iODBC
rewrites gpps.c from the first. I don't know if there's
such person in pg community.

regards,
Hiroshi Inoue

> > > Hmm I don't think the patch is proper.
> > > Someone must maintain ODBC stuff under unix.
> >
> > Oh, I thought you liked it. Is the new better than the old?
>
> Partly maybe. But the handling of odbcinst.ini seems
> to be different from its original intension.
>
> What I expect is that someone who knows well about iODBC
> rewrites gpps.c from the first. I don't know if there's
> such person in pg community.

Yes, certainly a rewrite would be ideal, but when I get a patch that
seems to make things marginally better, I apply it.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026