libpq thread-locking

Attached patch adds some error checking to the thread locking stuff in
libpq. Previously, if thread locking failed for some reason, we would
just fall through and do things without locking. This patch makes us
abort() instead. It's not the greatest thing probably, but our API
doesn't let us pass back return values...

Comments?

//Magnus


--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Magnus Hagander wrote:
> Attached patch adds some error checking to the thread locking stuff in
> libpq. Previously, if thread locking failed for some reason, we would
> just fall through and do things without locking. This patch makes us
> abort() instead. It's not the greatest thing probably, but our API
> doesn't let us pass back return values...

I have looked over the patch and it seems fine, though I am concerned
about the abort() case with no output. I realize stderr might be going
nowhere, but in fe-print.c we do an fprintf(stderr) for memory failures
so for consistency I think we should do the same here. If there is
concern about code bloat, I suggest a macro at the top of the file for
thread failure exits:

#define THEAD_FAILURE(str) \
do { \
fprintf(stderr, libpq_gettext("Thread failure: %s\n")); \
exit(1); \
} while(0)

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Bruce Momjian wrote:
> Magnus Hagander wrote:
> > Attached patch adds some error checking to the thread locking stuff in
> > libpq. Previously, if thread locking failed for some reason, we would
> > just fall through and do things without locking. This patch makes us
> > abort() instead. It's not the greatest thing probably, but our API
> > doesn't let us pass back return values...
>
> I have looked over the patch and it seems fine, though I am concerned
> about the abort() case with no output. I realize stderr might be going
> nowhere, but in fe-print.c we do an fprintf(stderr) for memory failures
> so for consistency I think we should do the same here. If there is
> concern about code bloat, I suggest a macro at the top of the file for
> thread failure exits:
>
> #define THEAD_FAILURE(str) \
> do { \
> fprintf(stderr, libpq_gettext("Thread failure: %s\n")); \
> exit(1); \
> } while(0)

Oh, this is Tom saying he doesn't like stderr and the added code lines
for failure:

http://archives.postgresql.org/pgsql...4/msg00254.php

I think the macro and consistency suggest doing as I outlined.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches