Postgres Pro
Downloads
Home   >   mailing lists

Thread: pgsql: Code review for recent libpq changes.

pgsql: Code review for recent libpq changes.

From
tgl@svr1.postgresql.org (Tom Lane)
Date:
Log Message:
-----------
Code review for recent libpq changes.  Be more careful about error
handling in SIGPIPE processing; avoid unnecessary pollution of application
link-symbol namespace; spell 'pointer to function' in the conventional
way.

Modified Files:
--------------
    pgsql/src/interfaces/libpq:
        fe-connect.c (r1.291 -> r1.292)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/fe-connect.c.diff?r1=1.291&r2=1.292)
        fe-print.c (r1.56 -> r1.57)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/fe-print.c.diff?r1=1.56&r2=1.57)
        fe-secure.c (r1.58 -> r1.59)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/fe-secure.c.diff?r1=1.58&r2=1.59)
        libpq-fe.h (r1.114 -> r1.115)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/libpq-fe.h.diff?r1=1.114&r2=1.115)
        libpq-int.h (r1.97 -> r1.98)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/libpq-int.h.diff?r1=1.97&r2=1.98)

Re: pgsql: Code review for recent libpq changes.

From
Bruce Momjian
Date:
Nice.  You were able to get the EPIPE checks in there.

---------------------------------------------------------------------------

Tom Lane wrote:
> Log Message:
> -----------
> Code review for recent libpq changes.  Be more careful about error
> handling in SIGPIPE processing; avoid unnecessary pollution of application
> link-symbol namespace; spell 'pointer to function' in the conventional
> way.
>
> Modified Files:
> --------------
>     pgsql/src/interfaces/libpq:
>         fe-connect.c (r1.291 -> r1.292)
>         (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/fe-connect.c.diff?r1=1.291&r2=1.292)
>         fe-print.c (r1.56 -> r1.57)
>         (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/fe-print.c.diff?r1=1.56&r2=1.57)
>         fe-secure.c (r1.58 -> r1.59)
>         (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/fe-secure.c.diff?r1=1.58&r2=1.59)
>         libpq-fe.h (r1.114 -> r1.115)
>         (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/libpq-fe.h.diff?r1=1.114&r2=1.115)
>         libpq-int.h (r1.97 -> r1.98)
>         (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/libpq-int.h.diff?r1=1.97&r2=1.98)
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: the planner will ignore your desire to choose an index scan if your
>       joining column's datatypes do not match
>

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

Re: pgsql: Code review for recent libpq changes.

From
Michael Fuhr
Date:
On Thu, Dec 02, 2004 at 11:20:24PM +0000, Tom Lane wrote:

> Code review for recent libpq changes.  Be more careful about error
> handling in SIGPIPE processing; avoid unnecessary pollution of application
> link-symbol namespace; spell 'pointer to function' in the conventional
> way.

Build failure on FreeBSD 4.10-STABLE:

fe-secure.c: In function `pqsecure_write':
fe-secure.c:433: `got_epipe' undeclared (first use in this function)

got_epipe is declared in an #ifdef ENABLE_THREAD_SAFETY block but
then used outside of such a block.

--
Michael Fuhr
http://www.fuhr.org/~mfuhr/

Re: pgsql: Code review for recent libpq changes.

From
Bruce Momjian
Date:
OK, fixed.  Patch attached.  Thanks.

---------------------------------------------------------------------------

Michael Fuhr wrote:
> On Thu, Dec 02, 2004 at 11:20:24PM +0000, Tom Lane wrote:
>
> > Code review for recent libpq changes.  Be more careful about error
> > handling in SIGPIPE processing; avoid unnecessary pollution of application
> > link-symbol namespace; spell 'pointer to function' in the conventional
> > way.
>
> Build failure on FreeBSD 4.10-STABLE:
>
> fe-secure.c: In function `pqsecure_write':
> fe-secure.c:433: `got_epipe' undeclared (first use in this function)
>
> got_epipe is declared in an #ifdef ENABLE_THREAD_SAFETY block but
> then used outside of such a block.
>
> --
> Michael Fuhr
> http://www.fuhr.org/~mfuhr/
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
>                http://archives.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
Index: interfaces/libpq/fe-secure.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.59
diff -c -c -r1.59 fe-secure.c
*** interfaces/libpq/fe-secure.c    2 Dec 2004 23:20:21 -0000    1.59
--- interfaces/libpq/fe-secure.c    3 Dec 2004 01:57:42 -0000
***************
*** 429,436 ****
--- 429,438 ----

                      if (n == -1)
                      {
+ #ifdef ENABLE_THREAD_SAFETY
                          if (SOCK_ERRNO == EPIPE)
                              got_epipe = true;
+ #endif
                          printfPQExpBuffer(&conn->errorMessage,
                                  libpq_gettext("SSL SYSCALL error: %s\n"),
                          SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));

Re: pgsql: Code review for recent libpq changes.

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> OK, fixed.  Patch attached.  Thanks.

Sigh, I'm an idiot ... sorry about that ...

            regards, tom lane