Обсуждение: Re: [PATCHES] PostgreSQL libraries - PThread Support, but
Lee, I have a question about this code:
char *pqStrerror(int errnum, char *strerrbuf, size_t buflen)
{
#if defined HAVE_STRERROR_R
/* reentrant strerror_r is available */
strerror_r(errnum, strerrbuf, buflen);
return strerrbuf;
#elif defined HAVE_NONPOSIX_STRERROR_R
/* broken (well early POSIX draft) strerror_r() which returns 'char *' */
return strerror_r(errnum, strerrbuf, buflen);
#else
/* no strerror_r() available, just use strerror */
return strerror(errnum);
#endif
}
Why do we have to care about HAVE_NONPOSIX_STRERROR_R? Can't we just
use the HAVE_STRERROR_R code in all cases?
---------------------------------------------------------------------------
Lee Kindness wrote:
Content-Description: message body text
> Patch attached, along with new libpq-reentrant.c and libpq-reentrant.h
> files for src/interfaces/libpq.
>
> Also at http://services.csl.co.uk/postgresql/
>
> Thanks, Lee.
>
> Lee Kindness writes:
> > Ok guys, I propose that the new libpq diff and 2 source files which
> > i'll soon send to pgsql-patches is applied to the source. This diff is
> > a cleaned up version of the previous version with the wrapper
> > functions moved out into their own file and more comments added. Also
> > the use of crypt_r() has been removed (not worth the effort), the
> > cpp defines have been renamed to be consistent with each other and
> > Tom's concerns with loose #defines has been partly addressed.
> >
> > This diff does not include any configure changes. I plan to tackle
> > this separately ASAP, and hopefully produce something more acceptable.
> >
> > I will add checks for appropriate compiler thread flags (for compiling
> > libpq, and alow the removal of #defines in libpq-reentrant.h), and
> > link flags & libs (for a planned threaded libpq test program and
> > renentrant ecpg library). If a thread environment is found then check
> > for the reentrant functions will be done.
> >
> > Looking at various open source projects configure.in files there seems
> > to be little commonality in the thread test macros (telp gratefully
> > accepted!), I currently think that something like the approach used by
> > glib is most suitable (switch on OS).
> >
> > All sound acceptable?
> >
> > Thanks, Lee.
> >
> > Peter Eisentraut writes:
> > > Lee Kindness writes:
> > >
> > > > Patches attached to make libpq thread-safe, now uses strerror_r(),
> > > > gethostbyname_r(), getpwuid_r() and crypt_r() where available. Where
> > > > strtok() was previously used strchr() is now used.
> > >
> > > AC_TRY_RUN tests are prohibited. Also, try to factor out some of these
> > > huge tests into separate macros and put them into config/c-library.m4.
> > > And it would be nice if there was some documentation about what was
> > > checked for. If you just want to check whether gethostbyname_r() has 5 or
> > > 6 arguments you can do that in half the space.
>
[ Attachment, skipping... ]
[ Attachment, skipping... ]
[ Attachment, skipping... ]
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Your call, but the "broken" call is in earlier glibc versions for
sure (if you're on a Linux box take a look in /usr/include - the
prototype is still there, may even get used depending on compiler
options!). I seem to remember compiling this on recent Solaris, HPUX,
Linux and AIX versions without hitting the "broken" version, but...
L.
Bruce Momjian writes:
>
> Lee, I have a question about this code:
>
> char *pqStrerror(int errnum, char *strerrbuf, size_t buflen)
> {
> #if defined HAVE_STRERROR_R
> /* reentrant strerror_r is available */
> strerror_r(errnum, strerrbuf, buflen);
> return strerrbuf;
> #elif defined HAVE_NONPOSIX_STRERROR_R
> /* broken (well early POSIX draft) strerror_r() which returns 'char *' */
> return strerror_r(errnum, strerrbuf, buflen);
> #else
> /* no strerror_r() available, just use strerror */
> return strerror(errnum);
> #endif
> }
>
> Why do we have to care about HAVE_NONPOSIX_STRERROR_R? Can't we just
> use the HAVE_STRERROR_R code in all cases?
>
> ---------------------------------------------------------------------------
>
> Lee Kindness wrote:
> Content-Description: message body text
>
> > Patch attached, along with new libpq-reentrant.c and libpq-reentrant.h
> > files for src/interfaces/libpq.
> >
> > Also at http://services.csl.co.uk/postgresql/
> >
> > Thanks, Lee.
> >
> > Lee Kindness writes:
> > > Ok guys, I propose that the new libpq diff and 2 source files which
> > > i'll soon send to pgsql-patches is applied to the source. This diff is
> > > a cleaned up version of the previous version with the wrapper
> > > functions moved out into their own file and more comments added. Also
> > > the use of crypt_r() has been removed (not worth the effort), the
> > > cpp defines have been renamed to be consistent with each other and
> > > Tom's concerns with loose #defines has been partly addressed.
> > >
> > > This diff does not include any configure changes. I plan to tackle
> > > this separately ASAP, and hopefully produce something more acceptable.
> > >
> > > I will add checks for appropriate compiler thread flags (for compiling
> > > libpq, and alow the removal of #defines in libpq-reentrant.h), and
> > > link flags & libs (for a planned threaded libpq test program and
> > > renentrant ecpg library). If a thread environment is found then check
> > > for the reentrant functions will be done.
> > >
> > > Looking at various open source projects configure.in files there seems
> > > to be little commonality in the thread test macros (telp gratefully
> > > accepted!), I currently think that something like the approach used by
> > > glib is most suitable (switch on OS).
> > >
> > > All sound acceptable?
> > >
> > > Thanks, Lee.
> > >
> > > Peter Eisentraut writes:
> > > > Lee Kindness writes:
> > > >
> > > > > Patches attached to make libpq thread-safe, now uses strerror_r(),
> > > > > gethostbyname_r(), getpwuid_r() and crypt_r() where available. Where
> > > > > strtok() was previously used strchr() is now used.
> > > >
> > > > AC_TRY_RUN tests are prohibited. Also, try to factor out some of these
> > > > huge tests into separate macros and put them into config/c-library.m4.
> > > > And it would be nice if there was some documentation about what was
> > > > checked for. If you just want to check whether gethostbyname_r() has 5 or
> > > > 6 arguments you can do that in half the space.
> >
>
> [ Attachment, skipping... ]
>
> [ Attachment, skipping... ]
>
> [ Attachment, skipping... ]
>
> --
> Bruce Momjian | http://candle.pha.pa.us
> pgman@candle.pha.pa.us | (610) 359-1001
> + If your life is a hard drive, | 13 Roberts Road
> + Christ can be your backup. | Newtown Square, Pennsylvania 19073
My point is why do we care whether it returns char * or nothing --- we
should just return strerrbuf in all cases.
---------------------------------------------------------------------------
Lee Kindness wrote:
> Your call, but the "broken" call is in earlier glibc versions for
> sure (if you're on a Linux box take a look in /usr/include - the
> prototype is still there, may even get used depending on compiler
> options!). I seem to remember compiling this on recent Solaris, HPUX,
> Linux and AIX versions without hitting the "broken" version, but...
>
> L.
>
> Bruce Momjian writes:
> >
> > Lee, I have a question about this code:
> >
> > char *pqStrerror(int errnum, char *strerrbuf, size_t buflen)
> > {
> > #if defined HAVE_STRERROR_R
> > /* reentrant strerror_r is available */
> > strerror_r(errnum, strerrbuf, buflen);
> > return strerrbuf;
> > #elif defined HAVE_NONPOSIX_STRERROR_R
> > /* broken (well early POSIX draft) strerror_r() which returns 'char *' */
> > return strerror_r(errnum, strerrbuf, buflen);
> > #else
> > /* no strerror_r() available, just use strerror */
> > return strerror(errnum);
> > #endif
> > }
> >
> > Why do we have to care about HAVE_NONPOSIX_STRERROR_R? Can't we just
> > use the HAVE_STRERROR_R code in all cases?
> >
> > ---------------------------------------------------------------------------
> >
> > Lee Kindness wrote:
> > Content-Description: message body text
> >
> > > Patch attached, along with new libpq-reentrant.c and libpq-reentrant.h
> > > files for src/interfaces/libpq.
> > >
> > > Also at http://services.csl.co.uk/postgresql/
> > >
> > > Thanks, Lee.
> > >
> > > Lee Kindness writes:
> > > > Ok guys, I propose that the new libpq diff and 2 source files which
> > > > i'll soon send to pgsql-patches is applied to the source. This diff is
> > > > a cleaned up version of the previous version with the wrapper
> > > > functions moved out into their own file and more comments added. Also
> > > > the use of crypt_r() has been removed (not worth the effort), the
> > > > cpp defines have been renamed to be consistent with each other and
> > > > Tom's concerns with loose #defines has been partly addressed.
> > > >
> > > > This diff does not include any configure changes. I plan to tackle
> > > > this separately ASAP, and hopefully produce something more acceptable.
> > > >
> > > > I will add checks for appropriate compiler thread flags (for compiling
> > > > libpq, and alow the removal of #defines in libpq-reentrant.h), and
> > > > link flags & libs (for a planned threaded libpq test program and
> > > > renentrant ecpg library). If a thread environment is found then check
> > > > for the reentrant functions will be done.
> > > >
> > > > Looking at various open source projects configure.in files there seems
> > > > to be little commonality in the thread test macros (telp gratefully
> > > > accepted!), I currently think that something like the approach used by
> > > > glib is most suitable (switch on OS).
> > > >
> > > > All sound acceptable?
> > > >
> > > > Thanks, Lee.
> > > >
> > > > Peter Eisentraut writes:
> > > > > Lee Kindness writes:
> > > > >
> > > > > > Patches attached to make libpq thread-safe, now uses strerror_r(),
> > > > > > gethostbyname_r(), getpwuid_r() and crypt_r() where available. Where
> > > > > > strtok() was previously used strchr() is now used.
> > > > >
> > > > > AC_TRY_RUN tests are prohibited. Also, try to factor out some of these
> > > > > huge tests into separate macros and put them into config/c-library.m4.
> > > > > And it would be nice if there was some documentation about what was
> > > > > checked for. If you just want to check whether gethostbyname_r() has 5 or
> > > > > 6 arguments you can do that in half the space.
> > >
> >
> > [ Attachment, skipping... ]
> >
> > [ Attachment, skipping... ]
> >
> > [ Attachment, skipping... ]
> >
> > --
> > Bruce Momjian | http://candle.pha.pa.us
> > pgman@candle.pha.pa.us | (610) 359-1001
> > + If your life is a hard drive, | 13 Roberts Road
> > + Christ can be your backup. | Newtown Square, Pennsylvania 19073
>
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian writes: > My point is why do we care whether it returns char * or nothing --- we > should just return strerrbuf in all cases. Ok, I see. Guess that is reasonable. L.
Lee, I have been looking at the code, and though the quoted function is
OK to avoid the non-posix case, it seems the later ones are a problem:
#if defined HAVE_NONPOSIX_GETPWUID_R
/* broken (well early POSIX draft) getpwuid_r() which returns
* 'struct passwd *' */
*result = getpwuid_r(uid, resultbuf, buffer, buflen);
#else
/* no getpwuid_r() available, just use getpwuid() */
*result = getpwuid(uid);
#endif /* HAVE_NONPOSIX_GETPWUID_R */
return( (*result == NULL) ? -1 : 0 );
In this case, what logic do we use? Seems you don't want to use
getpwuid_r if it exists, but only if getpwuid isn't thread safe, and I
have no idea how do to this. Can we just use the *_r versions if they
exist?
---------------------------------------------------------------------------
Bruce Momjian wrote:
>
> Lee, I have a question about this code:
>
> char *pqStrerror(int errnum, char *strerrbuf, size_t buflen)
> {
> #if defined HAVE_STRERROR_R
> /* reentrant strerror_r is available */
> strerror_r(errnum, strerrbuf, buflen);
> return strerrbuf;
> #elif defined HAVE_NONPOSIX_STRERROR_R
> /* broken (well early POSIX draft) strerror_r() which returns 'char *' */
> return strerror_r(errnum, strerrbuf, buflen);
> #else
> /* no strerror_r() available, just use strerror */
> return strerror(errnum);
> #endif
> }
>
> Why do we have to care about HAVE_NONPOSIX_STRERROR_R? Can't we just
> use the HAVE_STRERROR_R code in all cases?
>
> ---------------------------------------------------------------------------
>
> Lee Kindness wrote:
> Content-Description: message body text
>
> > Patch attached, along with new libpq-reentrant.c and libpq-reentrant.h
> > files for src/interfaces/libpq.
> >
> > Also at http://services.csl.co.uk/postgresql/
> >
> > Thanks, Lee.
> >
> > Lee Kindness writes:
> > > Ok guys, I propose that the new libpq diff and 2 source files which
> > > i'll soon send to pgsql-patches is applied to the source. This diff is
> > > a cleaned up version of the previous version with the wrapper
> > > functions moved out into their own file and more comments added. Also
> > > the use of crypt_r() has been removed (not worth the effort), the
> > > cpp defines have been renamed to be consistent with each other and
> > > Tom's concerns with loose #defines has been partly addressed.
> > >
> > > This diff does not include any configure changes. I plan to tackle
> > > this separately ASAP, and hopefully produce something more acceptable.
> > >
> > > I will add checks for appropriate compiler thread flags (for compiling
> > > libpq, and alow the removal of #defines in libpq-reentrant.h), and
> > > link flags & libs (for a planned threaded libpq test program and
> > > renentrant ecpg library). If a thread environment is found then check
> > > for the reentrant functions will be done.
> > >
> > > Looking at various open source projects configure.in files there seems
> > > to be little commonality in the thread test macros (telp gratefully
> > > accepted!), I currently think that something like the approach used by
> > > glib is most suitable (switch on OS).
> > >
> > > All sound acceptable?
> > >
> > > Thanks, Lee.
> > >
> > > Peter Eisentraut writes:
> > > > Lee Kindness writes:
> > > >
> > > > > Patches attached to make libpq thread-safe, now uses strerror_r(),
> > > > > gethostbyname_r(), getpwuid_r() and crypt_r() where available. Where
> > > > > strtok() was previously used strchr() is now used.
> > > >
> > > > AC_TRY_RUN tests are prohibited. Also, try to factor out some of these
> > > > huge tests into separate macros and put them into config/c-library.m4.
> > > > And it would be nice if there was some documentation about what was
> > > > checked for. If you just want to check whether gethostbyname_r() has 5 or
> > > > 6 arguments you can do that in half the space.
> >
>
> [ Attachment, skipping... ]
>
> [ Attachment, skipping... ]
>
> [ Attachment, skipping... ]
>
> --
> Bruce Momjian | http://candle.pha.pa.us
> pgman@candle.pha.pa.us | (610) 359-1001
> + If your life is a hard drive, | 13 Roberts Road
> + Christ can be your backup. | Newtown Square, Pennsylvania 19073
>
> ---------------------------(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) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073