Re: 7.1 odbc bug & patch

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