Re: Tablespaces - Mailing list pgsql-patches

From Neil Conway
Subject Re: Tablespaces
Date
Msg-id 40B59FBA.2080609@samurai.com
Whole thread Raw
In response to Tablespaces  (Gavin Sherry <swm@linuxworld.com.au>)
Responses Re: Tablespaces
List pgsql-patches
Gavin Sherry wrote:
> Attached is my latest patch implementing tablespaces. This has all the
> functionality I was planning for 7.5.

A few minor points I happened to notice while reading through the patch:

+  * To simply initialisation and XLog activity, have create and
maintain
+  * a symbolic link map in data/pg_tablespaces.

Grammar errors.

+ void
+ TblspcCreateDbspace(Oid tbloid)
+ {
+ #ifndef HAVE_SYMLINK
+     return;
+ #endif
+     struct stat st;
+     char        *dir;

If HAVE_SYMLINK is undefined, this is a syntax error (at least in
C89, which is what we ought to limit ourselves to). Similar problems
elsewhere in the same file (tablespc.c)

+     dir = (char *) palloc(strlen(DataDir) + 14 + 10 + 10 + 10 + 3 + 1);
+     sprintf(dir, "%s/pg_tablespaces/%u/%u", DataDir, tbloid,
+                 MyDatabaseId);

Is the length of that buffer right? At the least the addition is a
little weird (why are you adding 10 three times for two numeric
variables?) I noticed another buffer allocation (linkloc) that
looked dubious at first glance as well.

+     char        realnewpath[MAXPGPATH];

This is somewhat pedantic, but how do we know that MAXPGPATH >=
PATH_MAX (the minimum safe size of the second argument to
realpath(), at least on my local system)?

-Neil


pgsql-patches by date:

Previous
From: Christopher Kings-Lynne
Date:
Subject: Re: Tablespaces
Next
From: "Thomas Hallgren"
Date:
Subject: Re: pg_ctl.c