Preliminary patch for FRONTEND
As many know, the FRONTEND usage of /src/port is very fragile. It
requires every binary that uses certain libpgport object files to create
its own version, which is very fragile, and could easily break if a
function call is added in a subrelease, especially on certain ports.
The following patch addresses this issue by making libpgport usable
unchanged by client applications, and makes a special server version for
the backend. This is much less fragile than trying to make sure you
hit every single binary that uses libpgport.
I originally tried abstracting out the #ifdef FRONTEND defines in pgport
but found that that wasn't easily done so making separate libraries was
cleaner.
--
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 19073Attachments:
/pgpatches/pgporttext/plainDownload+133-145
Pasto:
*** src/bin/pg_config/Makefile 1 Aug 2004 06:56:38 -0000 1.8
--- src/bin/pg_config/Makefile 1 Oct 2004 04:04:06 -0000
***************
*** 1,18 ****
[...]
--- 1,23 ----
!
#-------------------------------------------------------------------------
! #
! # Makefile for src/bin/pg_controldata
! #
! # Copyright (c) 1998-2002, PostgreSQL Global Development Group
! #
! # $PostgreSQL: pgsql-server/src/bin/pg_controldata/Makefile,v 1.9
2004/05/26 17:24:01 tgl Exp $
! #
!
#-------------------------------------------------------------------------s/pg_controldata/pg_config/ (and maybe fix the copyright date range?)
*** src/port/Makefile 28 Aug 2004 22:55:06 -0000 1.18
--- src/port/Makefile 1 Oct 2004 04:04:11 -0000
***************
*** 48,51 ****
echo "#define LOCALEDIR \"$(localedir)\"" >>$@ clean distclean maintainer-clean:
! rm -f libpgport.a $(LIBOBJS) pg_config_paths.h
--- 68,71 ----
echo "#define LOCALEDIR \"$(localedir)\"" >>$@ clean distclean maintainer-clean:
! rm -f libpgport.a $(LIBOBJS) $(LIBOBJS_SRV) pg_config_paths.h
"clean" should remove libpgport_srv.a as well, shouldn't it?
*** src/port/getaddrinfo.c 28 Sep 2004 00:07:01 -0000 1.15
--- src/port/getaddrinfo.c 1 Oct 2004 04:04:12 -0000
***************
*** 26,31 ****
--- 26,32 ----
#include <netdb.h>
#include <netinet/in.h>
#include <arpa/inet.h>
+ #include <rpc/types.h>
#endif#include "getaddrinfo.h"
What is this change for?
-Neil
Bruce Momjian <pgman@candle.pha.pa.us> writes:
The following patch addresses this issue by making libpgport usable
unchanged by client applications, and makes a special server version for
the backend.
This raises some alarm bells for me. Why does a "port support" library
need to distinguish whether it is running in frontend or backend?
Certainly the standard Unix library calls that it is purporting to
replace do not care.
Also, the patch does both more and less than I'd expect from your
description. Why are there no changes at all to C code? And how can
you just remove, eg, dirmod.c and exec.c from pg_resetxlog?
regards, tom lane
The following patch addresses this issue by making libpgport usable
unchanged by client applications, and makes a specialserver version
for the backend.
This raises some alarm bells for me. Why does a "port
support" library need to distinguish whether it is running in
frontend or backend?
Just from the problems I've seen with several modules - ereport().
Several functions use ereport() if !FRONTEND and something else if
FRONTEND.
I've seen this problem several times when trying to compile things "out
of sync". The main issue is that the port stuff behave differently,
certainly. I originally thought the deal was that anything that relied
on backend stuff would go in backend/port, but there are (and has been
since before I started looking at it) several files in /port/ taht rely
heavily on functions and variables in the backend.
//Magnus
Import Notes
Resolved by subject fallback
Magnus Hagander wrote:
The following patch addresses this issue by making libpgport usable
unchanged by client applications, and makes a specialserver version
for the backend.
This raises some alarm bells for me. Why does a "port
support" library need to distinguish whether it is running in
frontend or backend?Just from the problems I've seen with several modules - ereport().
Several functions use ereport() if !FRONTEND and something else if
FRONTEND.I've seen this problem several times when trying to compile things "out
of sync". The main issue is that the port stuff behave differently,
certainly. I originally thought the deal was that anything that relied
on backend stuff would go in backend/port, but there are (and has been
since before I started looking at it) several files in /port/ taht rely
heavily on functions and variables in the backend.
The basic issue is ereport and memory allocation for dirmod, ereport for
exec.c, thread-safety for getaddrinfo and thread.c. I can't think of
how to cleanly abstract them so the backend and libpq have compatibile
versions to call so I can use the same object file, so I made new
libraries.
And as Magnus has pointed out, they do get out of sync too easily
without this cleanup.
--
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 19073Neil Conway wrote:
Pasto:
*** src/bin/pg_config/Makefile 1 Aug 2004 06:56:38 -0000 1.8 --- src/bin/pg_config/Makefile 1 Oct 2004 04:04:06 -0000 *************** *** 1,18 **** [...] --- 1,23 ---- ! #------------------------------------------------------------------------- ! # ! # Makefile for src/bin/pg_controldata ! # ! # Copyright (c) 1998-2002, PostgreSQL Global Development Group ! # ! # $PostgreSQL: pgsql-server/src/bin/pg_controldata/Makefile,v 1.9 2004/05/26 17:24:01 tgl Exp $ ! # ! #-------------------------------------------------------------------------s/pg_controldata/pg_config/ (and maybe fix the copyright date range?)
*** src/port/Makefile 28 Aug 2004 22:55:06 -0000 1.18 --- src/port/Makefile 1 Oct 2004 04:04:11 -0000 *************** *** 48,51 **** echo "#define LOCALEDIR \"$(localedir)\"" >>$@clean distclean maintainer-clean: ! rm -f libpgport.a $(LIBOBJS) pg_config_paths.h --- 68,71 ---- echo "#define LOCALEDIR \"$(localedir)\"" >>$@clean distclean maintainer-clean:
! rm -f libpgport.a $(LIBOBJS) $(LIBOBJS_SRV) pg_config_paths.h"clean" should remove libpgport_srv.a as well, shouldn't it?
Fixed, thanks.
*** src/port/getaddrinfo.c 28 Sep 2004 00:07:01 -0000 1.15 --- src/port/getaddrinfo.c 1 Oct 2004 04:04:12 -0000 *************** *** 26,31 **** --- 26,32 ---- #include <netdb.h> #include <netinet/in.h> #include <arpa/inet.h> + #include <rpc/types.h> #endif#include "getaddrinfo.h"
What is this change for?
My OS couldn't compile getaddrinfo when I tried, though my OS doesn't
need getaddrinfo so maybe we shouldn't make that change. Comments?
--
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 19073Tom Lane wrote:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
The following patch addresses this issue by making libpgport usable
unchanged by client applications, and makes a special server version for
the backend.This raises some alarm bells for me. Why does a "port support" library
need to distinguish whether it is running in frontend or backend?
Certainly the standard Unix library calls that it is purporting to
replace do not care.
Already described --- ereport, memory allocation, and thread safety.
Also, the patch does both more and less than I'd expect from your
description. Why are there no changes at all to C code? And how can
you just remove, eg, dirmod.c and exec.c from pg_resetxlog?
Basically libpgport now is compiled for the frontend so it just gets it
from the link line rather than build its own.
--
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 19073Bruce Momjian <pgman@candle.pha.pa.us> writes:
+ #include <rpc/types.h>
What is this change for?
My OS couldn't compile getaddrinfo when I tried, though my OS doesn't
need getaddrinfo so maybe we shouldn't make that change. Comments?
Don't put it in. That looks like the sort of file that isn't even going
to exist on a lot of platforms.
regards, tom lane
Tom Lane wrote:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
+ #include <rpc/types.h>
What is this change for?
My OS couldn't compile getaddrinfo when I tried, though my OS doesn't
need getaddrinfo so maybe we shouldn't make that change. Comments?Don't put it in. That looks like the sort of file that isn't even going
to exist on a lot of platforms.
OK, removed.
--
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 19073Bruce Momjian wrote:
As many know, the FRONTEND usage of /src/port is very fragile. It
requires every binary that uses certain libpgport object files to create
its own version, which is very fragile, and could easily break if a
function call is added in a subrelease, especially on certain ports.The following patch addresses this issue by making libpgport usable
unchanged by client applications, and makes a special server version for
the backend. This is much less fragile than trying to make sure you
hit every single binary that uses libpgport.I originally tried abstracting out the #ifdef FRONTEND defines in pgport
but found that that wasn't easily done so making separate libraries was
cleaner.
Here is an updated version of the patch:
ftp://candle.pha.pa.us/pub/postgresql/mypatches
I made the suggested modifications and added more comments.
--
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