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


---------------------------(end of broadcast)--------------------------- TIP 4: Don't 'kill -9' the postmaster

Reply via email to