On 09/03/2012 03:22 PM, Tom Lane wrote:
Andrew Dunstan <and...@dunslane.net> writes:
This time with a patch.
Nitpicky gripe: "fix_path" is a mighty generic name. How about
"fix_path_for_windows" or something like that? I don't think I'd
mark it inline, either.
More generally, the behavior of combining two (maybe) filename segments
seems overcomplicated and unnecessary. Why not just have it take *one*
argument and back-slashify that, without the concatenation behavior?
Then you'd have two calls instead of one at some of the call sites,
but that doesn't seem like much of a loss. The malloc'd strings are
getting leaked anyway. The function itself would reduce to pg_strdup
and a backslashification loop. Also, you could turn it into a complete
no-op (not even pg_strdup) on non-Windows.
OK, revised patch attached.
cheers
andrew
diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
index efb080b..d965fa8 100644
--- a/contrib/pg_upgrade/check.c
+++ b/contrib/pg_upgrade/check.c
@@ -23,6 +23,35 @@ static void check_for_reg_data_type_usage(ClusterInfo *cluster);
static void get_bin_version(ClusterInfo *cluster);
+/*
+ * fix_path_separator
+ * For non-Windows, just return the argument.
+ * For Windows convert any forward slash to a backslash
+ * such as is suitable for arguments to builtin commands
+ * like RMDIR and DEL.
+ */
+static char *fix_path_separator(char *path)
+{
+#ifdef WIN32
+
+ char *result;
+ char *c;
+
+ result = pg_strdup(path);
+
+ for (c = result; *c != '\0'; c++)
+ if (*c == '/')
+ *c = '\\';
+
+ return result;
+
+#else
+
+ return path;
+
+#endif
+}
+
void
output_check_banner(bool *live_check)
{
@@ -544,7 +573,7 @@ create_script_for_old_cluster_deletion(char **deletion_script_file_name)
#endif
/* delete old cluster's default tablespace */
- fprintf(script, RMDIR_CMD " %s\n", old_cluster.pgdata);
+ fprintf(script, RMDIR_CMD " %s\n", fix_path_separator(old_cluster.pgdata));
/* delete old cluster's alternate tablespaces */
for (tblnum = 0; tblnum < os_info.num_tablespaces; tblnum++)
@@ -561,14 +590,17 @@ create_script_for_old_cluster_deletion(char **deletion_script_file_name)
fprintf(script, "\n");
/* remove PG_VERSION? */
if (GET_MAJOR_VERSION(old_cluster.major_version) <= 804)
- fprintf(script, RM_CMD " %s%s/PG_VERSION\n",
- os_info.tablespaces[tblnum], old_cluster.tablespace_suffix);
+ fprintf(script, RM_CMD " %s%s%cPG_VERSION\n",
+ fix_path_separator(os_info.tablespaces[tblnum]),
+ fix_path_separator(old_cluster.tablespace_suffix),
+ PATH_SEPARATOR);
for (dbnum = 0; dbnum < old_cluster.dbarr.ndbs; dbnum++)
{
- fprintf(script, RMDIR_CMD " %s%s/%d\n",
- os_info.tablespaces[tblnum], old_cluster.tablespace_suffix,
- old_cluster.dbarr.dbs[dbnum].db_oid);
+ fprintf(script, RMDIR_CMD " %s%s%c%d\n",
+ fix_path_separator(os_info.tablespaces[tblnum]),
+ fix_path_separator(old_cluster.tablespace_suffix),
+ PATH_SEPARATOR, old_cluster.dbarr.dbs[dbnum].db_oid);
}
}
else
@@ -578,7 +610,8 @@ create_script_for_old_cluster_deletion(char **deletion_script_file_name)
* or a version-specific subdirectory.
*/
fprintf(script, RMDIR_CMD " %s%s\n",
- os_info.tablespaces[tblnum], old_cluster.tablespace_suffix);
+ fix_path_separator(os_info.tablespaces[tblnum]),
+ fix_path_separator(old_cluster.tablespace_suffix));
}
fclose(script);
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
index 7b19d91..b57da53 100644
--- a/contrib/pg_upgrade/pg_upgrade.h
+++ b/contrib/pg_upgrade/pg_upgrade.h
@@ -72,6 +72,7 @@ extern char *output_files[];
#define pg_copy_file copy_file
#define pg_mv_file rename
#define pg_link_file link
+#define PATH_SEPARATOR '/'
#define RM_CMD "rm -f"
#define RMDIR_CMD "rm -rf"
#define SCRIPT_EXT "sh"
@@ -81,6 +82,7 @@ extern char *output_files[];
#define pg_mv_file pgrename
#define pg_link_file win32_pghardlink
#define sleep(x) Sleep(x * 1000)
+#define PATH_SEPARATOR '\\'
#define RM_CMD "DEL /q"
#define RMDIR_CMD "RMDIR /s/q"
#define SCRIPT_EXT "bat"
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
