Re: [PATCH] Make various variables read-only (const)

On Fri, Dec 20, 2013 at 12:01 PM, Oskari Saarenmaa <os(at)ohmu(dot)fi> wrote:
> This allows the variables to be moved from .data to .rodata section which
> means that more data can be shared by processes and makes sure that nothing
> can accidentally overwrite the read-only definitions. On a x86-64 Linux
> system this moves roughly 9 kilobytes of previously writable data to the
> read-only data segment in the backend and 4 kilobytes in libpq.
>
> https://github.com/saaros/postgres/compare/constify
>
> 24 files changed, 108 insertions(+), 137 deletions(-)

This sounds like a broadly good thing, but I've had enough painful
experiences with const to be a little wary. And how much does this
really affect data sharing? Doesn't copy-on-write do the same thing
for writable data? Could we get most of the benefit by const-ifying
one or two large data structures and forget the rest?

Other comments:

- The first hunk of the patch mutilates the comment it modifies for no
apparent reason. Please revert.

- Why change the API of transformRelOptions()?

-#define DEF_ENC2NAME(name, codepage) { #name, PG_##name }
+/* The extra NUL-terminator will make sure a warning is raised if the
+ * storage space for name is too small, otherwise when strlen(name) ==
+ * sizeof(pg_enc2name.name) the NUL-terminator would be silently dropped.
+ */
+#define DEF_ENC2NAME(name, codepage) { #name "\0", PG_##name }

- The above hunk is not related to the primary purpose of this patch.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company