7.1 odbc bug & patch

Started by Dave Bodenstababout 25 years ago12 messagespatches
Jump to latest
#1Dave Bodenstab
imdave@mcs.net

Your name : Dave Bodenstab
Your email address : imdave@mcs.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);

#2Bruce Momjian
bruce@momjian.us
In reply to: Dave Bodenstab (#1)
Re: 7.1 odbc bug & patch

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@mcs.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@postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#3Bruce Momjian
bruce@momjian.us
In reply to: Dave Bodenstab (#1)
Re: 7.1 odbc bug & patch

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@mcs.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@postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#4Hiroshi Inoue
Inoue@tpf.co.jp
In reply to: Bruce Momjian (#3)
RE: 7.1 odbc bug & patch

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

#5Bruce Momjian
bruce@momjian.us
In reply to: Hiroshi Inoue (#4)
Re: 7.1 odbc bug & patch

-----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@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#6Bruce Momjian
bruce@momjian.us
In reply to: Dave Bodenstab (#1)
Re: 7.1 odbc bug & patch

Thanks. Patch applied.

Your name : Dave Bodenstab
Your email address : imdave@mcs.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@postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#7Hiroshi Inoue
Inoue@tpf.co.jp
In reply to: Bruce Momjian (#5)
Re: 7.1 odbc bug & patch

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

#8Bruce Momjian
bruce@momjian.us
In reply to: Hiroshi Inoue (#7)
Re: 7.1 odbc bug & patch

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@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#9Hiroshi Inoue
Inoue@tpf.co.jp
In reply to: Bruce Momjian (#8)
Re: 7.1 odbc bug & patch

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

#10Bruce Momjian
bruce@momjian.us
In reply to: Hiroshi Inoue (#9)
Re: 7.1 odbc bug & patch

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@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#11Dave Bodenstab
imdave@mcs.net
In reply to: Bruce Momjian (#10)
Re: 7.1 odbc bug & patch

As long as I'm on the cc list here... let me add $.02

What little documentation I found was in:

postgresql-7.1/doc/postgres/odbc-install.html

In section 7.2 Installation, it states:

The installation-wide configuration file odbcinst.ini will be
installed into the directory /usr/local/pgsql/etc/, or equivalent,
depending on what --prefix and/or --sysconfdir options you supplied
to configure. Since this file can also be shared between different
ODBC drivers you can also install it in a shared location. To do
that, override the location of this file with the --with-odbcinst
option.

This led *me* to believe that when I configure and install postgress
with:

--enable-odbc \
--with-odbcinst=/usr/local/etc \

then postgress would actually *use* /usr/local/etc/odbcinst.ini.
It didn't. There was no mention of looking in the current directory
or looking in $HOME. When I ktrace'd my test program, I found that
an odbcinst.ini file *was* being searched for, but only in . and
$HOME. Finally tracking it down, I found the reason in gpps.c (and
found questionable and buggy code also.) In fact, the only reference
to the installed odbcinst.ini (ODBCINST_INI) is in dlg_specific.c,
in code that is never called unless WIN32 is #defined, and even if
#defined, it's attempting to *update* the file. It was obvious
that the code did not match the available doc's. Since it wasn't
obvious whether or not odbcinst.ini should be searched for in .
and $HOME, I left that logic alone. I only added a final search
for ODBCINST_INI which, finally, makes it do what the documentation
states that it should. (The buggy code is another story... that
should be removed/fixed regardless of whether the search is
augmented.) End of story... the patch got things working.

There are, perhaps, other problems... I don't know. I only played
with this stuff in an attempt to learn about ``odbc'' stuff. When
I ktrace'd libiodbc, it wanted its own "odbc.ini" file. I would
think that they would use the same file name... but since I don't
have any doc's on how things are supposed to work, I have not investigated
further. In any event, both libiodbc and tclodbc require database
specific libraries -- for postgres, src/interfaces/odbc/ -- so I
would expect that postgres would supply working (with respect to
its own documentation) code.

Dave Bodenstab

#12Hiroshi Inoue
Inoue@tpf.co.jp
In reply to: Dave Bodenstab (#11)
Re: 7.1 odbc bug & patch

Dave Bodenstab wrote:

As long as I'm on the cc list here... let me add $.02

What little documentation I found was in:

postgresql-7.1/doc/postgres/odbc-install.html

In section 7.2 Installation, it states:

The installation-wide configuration file odbcinst.ini will be
installed into the directory /usr/local/pgsql/etc/, or equivalent,
depending on what --prefix and/or --sysconfdir options you supplied
to configure. Since this file can also be shared between different
ODBC drivers you can also install it in a shared location. To do
that, override the location of this file with the --with-odbcinst
option.

Could someone tell me where the above doc come from ?
Certainly both windows-ODBC and unixODBC need odbcinst.ini
but I couldn't find *odbcinst* in any iODBC doc/src.
In fact psqlodbc driver works with iODBC without odbcinst.ini.
I'm not familiar with ODBC under unix and am happy if someone
explains it.

This led *me* to believe that when I configure and install postgress
with:

--enable-odbc \
--with-odbcinst=/usr/local/etc \

then postgress would actually *use* /usr/local/etc/odbcinst.ini.
It didn't. There was no mention of looking in the current directory
or looking in $HOME. When I ktrace'd my test program, I found that
an odbcinst.ini file *was* being searched for, but only in . and
$HOME. Finally tracking it down, I found the reason in gpps.c (and
found questionable and buggy code also.) In fact, the only reference
to the installed odbcinst.ini (ODBCINST_INI) is in dlg_specific.c,
in code that is never called unless WIN32 is #defined, and even if
#defined, it's attempting to *update* the file.

No psqlodbc really uses ODBCINST_INI under Windows and probably
under unix also. I could see
getGlobalDefaults(DBMS_NAME, ODBCINST_INI, FALSE)
in psqlodbc.c and environ.c.

[snip]

There are, perhaps, other problems... I don't know. I only played
with this stuff in an attempt to learn about ``odbc'' stuff. When
I ktrace'd libiodbc, it wanted its own "odbc.ini" file. I would
think that they would use the same file name... but since I don't
have any doc's on how things are supposed to work, I have not investigated
further.

Yes you are right.
There must be a documentation about how to let
the psqlodbc driver work with iDOBC though I don't
know who could do it. And someone should improve gpps.c.

Thanks,
Hiroshi Inoue