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);