By an odd coincidence, I also decided to try to const-ify libpq
recently, and noticed this thread as I was cleaning up my patch for
submission. For what it's worth, I've attached my patch to this message.
It doesn't move as much data into the text segment as Oskari Saarenmaa's
patch does, but it is less intrusive (simply adding const modifiers here
and there). I just went after the low-hanging fruit in libpq.

As an aside, it might make sense to make pg_encname_tbl and
pg_encname_tbl_sz static, since as far as I can tell those symbols are
never used outside of encnames.c nor are they likely to be. I didn't
make that change in this patch though.

On Mon, Dec 23, 2013, Robert Haas <robertmh...@gmail.com> wrote:
> And how much does this really affect data sharing?  Doesn't copy-on-write do 
> the same thing for writable data?

It can have a surprisingly large effect if read-only data gets
intermixed on pages with actual read-write data and can get COWd
unnecessarily.

My motivation, though, was more about code correctness than memory
sharing, though sharing is a nice benefit--- I was examining unexpected
symbols in .data/.bss in case they represented a thread-safety problem
for my program linked against libpq. (They didn't, FWIW, except for the
known and documented issue with PQoidStatus(). Not that I really
expected to find a problem in libpq, but marking those structures const
makes it nice and clear that they're not mutable.)

diff --git a/src/backend/utils/mb/encnames.c b/src/backend/utils/mb/encnames.c
index 772d4a5..3f8c592 100644
--- a/src/backend/utils/mb/encnames.c
+++ b/src/backend/utils/mb/encnames.c
@@ -29,7 +29,7 @@
  * Karel Zak, Aug 2001
  * ----------
  */
-pg_encname     pg_encname_tbl[] =
+const pg_encname       pg_encname_tbl[] =
 {
        {
                "abc", PG_WIN1258
@@ -291,7 +291,7 @@ pg_encname  pg_encname_tbl[] =
        }                                                       /* last */
 };
 
-unsigned int pg_encname_tbl_sz = \
+const unsigned int pg_encname_tbl_sz = \
 sizeof(pg_encname_tbl) / sizeof(pg_encname_tbl[0]) - 1;
 
 /* ----------
@@ -304,7 +304,7 @@ sizeof(pg_encname_tbl) / sizeof(pg_encname_tbl[0]) - 1;
 #else
 #define DEF_ENC2NAME(name, codepage) { #name, PG_##name, codepage }
 #endif
-pg_enc2name pg_enc2name_tbl[] =
+const pg_enc2name pg_enc2name_tbl[] =
 {
        DEF_ENC2NAME(SQL_ASCII, 0),
        DEF_ENC2NAME(EUC_JP, 20932),
@@ -356,7 +356,7 @@ pg_enc2name pg_enc2name_tbl[] =
  * This covers all encodings except MULE_INTERNAL, which is alien to gettext.
  * ----------
  */
-pg_enc2gettext pg_enc2gettext_tbl[] =
+const pg_enc2gettext pg_enc2gettext_tbl[] =
 {
        {PG_SQL_ASCII, "US-ASCII"},
        {PG_UTF8, "UTF-8"},
@@ -469,11 +469,11 @@ clean_encoding_name(const char *key, char *newkey)
  * Search encoding by encoding name
  * ----------
  */
-pg_encname *
+const pg_encname *
 pg_char_to_encname_struct(const char *name)
 {
        unsigned int nel = pg_encname_tbl_sz;
-       pg_encname *base = pg_encname_tbl,
+       const pg_encname *base = pg_encname_tbl,
                           *last = base + nel - 1,
                           *position;
        int                     result;
@@ -521,7 +521,7 @@ pg_char_to_encname_struct(const char *name)
 int
 pg_char_to_encoding(const char *name)
 {
-       pg_encname *p;
+       const pg_encname *p;
 
        if (!name)
                return -1;
@@ -545,7 +545,7 @@ pg_encoding_to_char(int encoding)
 {
        if (PG_VALID_ENCODING(encoding))
        {
-               pg_enc2name *p = &pg_enc2name_tbl[encoding];
+               const pg_enc2name *p = &pg_enc2name_tbl[encoding];
 
                Assert(encoding == p->encoding);
                return p->name;
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index 6d1cd8e..08440e9 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -55,9 +55,9 @@ static FmgrInfo *ToClientConvProc = NULL;
 /*
  * These variables track the currently-selected encodings.
  */
-static pg_enc2name *ClientEncoding = &pg_enc2name_tbl[PG_SQL_ASCII];
-static pg_enc2name *DatabaseEncoding = &pg_enc2name_tbl[PG_SQL_ASCII];
-static pg_enc2name *MessageEncoding = &pg_enc2name_tbl[PG_SQL_ASCII];
+static const pg_enc2name *ClientEncoding = &pg_enc2name_tbl[PG_SQL_ASCII];
+static const pg_enc2name *DatabaseEncoding = &pg_enc2name_tbl[PG_SQL_ASCII];
+static const pg_enc2name *MessageEncoding = &pg_enc2name_tbl[PG_SQL_ASCII];
 
 /*
  * During backend startup we can't set client encoding because we (a)
diff --git a/src/backend/utils/mb/wchar.c b/src/backend/utils/mb/wchar.c
index 45bc3c1..6d03a10 100644
--- a/src/backend/utils/mb/wchar.c
+++ b/src/backend/utils/mb/wchar.c
@@ -1720,7 +1720,7 @@ pg_eucjp_increment(unsigned char *charptr, int length)
  * XXX must be sorted by the same order as enum pg_enc (in mb/pg_wchar.h)
  *-------------------------------------------------------------------
  */
-pg_wchar_tbl pg_wchar_table[] = {
+const pg_wchar_tbl pg_wchar_table[] = {
        {pg_ascii2wchar_with_len, pg_wchar2single_with_len, pg_ascii_mblen, 
pg_ascii_dsplen, pg_ascii_verifier, 1}, /* PG_SQL_ASCII */
        {pg_eucjp2wchar_with_len, pg_wchar2euc_with_len, pg_eucjp_mblen, 
pg_eucjp_dsplen, pg_eucjp_verifier, 3},        /* PG_EUC_JP */
        {pg_euccn2wchar_with_len, pg_wchar2euc_with_len, pg_euccn_mblen, 
pg_euccn_dsplen, pg_euccn_verifier, 2},        /* PG_EUC_CN */
diff --git a/src/include/mb/pg_wchar.h b/src/include/mb/pg_wchar.h
index d255c64..deabbac 100644
--- a/src/include/mb/pg_wchar.h
+++ b/src/include/mb/pg_wchar.h
@@ -307,12 +307,12 @@ typedef enum pg_enc
  */
 typedef struct pg_encname
 {
-       char       *name;
+       const char         *name;
        pg_enc          encoding;
 } pg_encname;
 
-extern pg_encname pg_encname_tbl[];
-extern unsigned int pg_encname_tbl_sz;
+extern const pg_encname pg_encname_tbl[];
+extern const unsigned int pg_encname_tbl_sz;
 
 /*
  * Careful:
@@ -322,14 +322,14 @@ extern unsigned int pg_encname_tbl_sz;
  */
 typedef struct pg_enc2name
 {
-       char       *name;
+       const char         *name;
        pg_enc          encoding;
 #ifdef WIN32
        unsigned        codepage;               /* codepage for WIN32 */
 #endif
 } pg_enc2name;
 
-extern pg_enc2name pg_enc2name_tbl[];
+extern const pg_enc2name pg_enc2name_tbl[];
 
 /*
  * Encoding names for gettext
@@ -340,7 +340,7 @@ typedef struct pg_enc2gettext
        const char *name;
 } pg_enc2gettext;
 
-extern pg_enc2gettext pg_enc2gettext_tbl[];
+extern const pg_enc2gettext pg_enc2gettext_tbl[];
 
 /*
  * pg_wchar stuff
@@ -373,7 +373,7 @@ typedef struct
        int                     maxmblen;               /* max bytes for a char 
in this encoding */
 } pg_wchar_tbl;
 
-extern pg_wchar_tbl pg_wchar_table[];
+extern const pg_wchar_tbl pg_wchar_table[];
 
 /*
  * UTF-8 to local code conversion map
@@ -441,7 +441,7 @@ extern int  pg_valid_server_encoding_id(int encoding);
  * Remaining functions are not considered part of libpq's API, though many
  * of them do exist inside libpq.
  */
-extern pg_encname *pg_char_to_encname_struct(const char *name);
+extern const pg_encname *pg_char_to_encname_struct(const char *name);
 
 extern int     pg_mb2wchar(const char *from, pg_wchar *to);
 extern int     pg_mb2wchar_with_len(const char *from, pg_wchar *to, int len);
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index d88c752..c1f9755 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -1594,7 +1594,7 @@ close_SSL(PGconn *conn)
  * return NULL if it doesn't recognize the error code.  We don't
  * want to return NULL ever.
  */
-static char ssl_nomem[] = "out of memory allocating error description";
+static const char ssl_nomem[] = "out of memory allocating error description";
 
 #define SSL_ERR_LEN 128
 
@@ -1607,7 +1607,7 @@ SSLerrmessage(void)
 
        errbuf = malloc(SSL_ERR_LEN);
        if (!errbuf)
-               return ssl_nomem;
+               return (char *)ssl_nomem;
        errcode = ERR_get_error();
        if (errcode == 0)
        {
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to