Fix a number of places that were making file-type tests infelicitously.

tglsfdc · tglsfdc · commit c5f11f9d1996 · 2008-03-31T01:31:43.000Z
The places that did, eg,
	(statbuf.st_mode &amp; S_IFMT) == S_IFDIR
were correct, but there is no good reason not to use S_ISDIR() instead,
especially when that's what the other 90% of our code does.  The places
that did, eg,
	(statbuf.st_mode &amp; S_IFDIR)
were flat out *wrong* and would fail in various platform-specific ways,
eg a symlink could be mistaken for a regular file on most Unixen.

The actual impact of this is probably small, since the problem cases
seem to always involve symlinks or sockets, which are unlikely to be
found in the directories that PG code might be scanning.  But it's
clearly trouble waiting to happen, so patch all the way back anyway.
(There seem to be no occurrences of the mistake in 7.4.)
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
@@ -5,7 +5,7 @@
  * Copyright (c) 2002-2008, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/dbsize.c,v 1.17 2008/03/25 22:42:44 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/dbsize.c,v 1.18 2008/03/31 01:31:43 tgl Exp $
  *
  */
 
@@ -209,7 +209,7 @@ calculate_tablespace_size(Oid tblspcOid)
 						 errmsg("could not stat file \"%s\": %m", pathname)));
 		}
 
-		if (fst.st_mode & S_IFDIR)
+		if (S_ISDIR(fst.st_mode))
 			totalsize += db_dir_size(pathname);
 
 		totalsize += fst.st_size;
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
@@ -9,7 +9,7 @@
  * Author: Andreas Pflug <pgadmin@pse-consulting.de>
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/genfile.c,v 1.18 2008/03/25 22:42:44 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/genfile.c,v 1.19 2008/03/31 01:31:43 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -198,7 +198,7 @@ pg_stat_file(PG_FUNCTION_ARGS)
 	isnull[3] = true;
 	values[4] = TimestampTzGetDatum(time_t_to_timestamptz(fst.st_ctime));
 #endif
-	values[5] = BoolGetDatum(fst.st_mode & S_IFDIR);
+	values[5] = BoolGetDatum(S_ISDIR(fst.st_mode));
 
 	tuple = heap_form_tuple(tupdesc, values, isnull);
 
diff --git a/src/port/copydir.c b/src/port/copydir.c
@@ -11,7 +11,7 @@
  *	as a service.
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/port/copydir.c,v 1.21 2008/01/01 19:46:00 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/port/copydir.c,v 1.22 2008/03/31 01:31:43 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -80,13 +80,13 @@ copydir(char *fromdir, char *todir, bool recurse)
 					(errcode_for_file_access(),
 					 errmsg("could not stat file \"%s\": %m", fromfile)));
 
-		if (fst.st_mode & S_IFDIR)
+		if (S_ISDIR(fst.st_mode))
 		{
 			/* recurse to handle subdirectories */
 			if (recurse)
 				copydir(fromfile, tofile, true);
 		}
-		else if (fst.st_mode & S_IFREG)
+		else if (S_ISREG(fst.st_mode))
 			copy_file(fromfile, tofile);
 	}
 
diff --git a/src/port/exec.c b/src/port/exec.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/port/exec.c,v 1.58 2008/02/29 15:31:33 mha Exp $
+ *	  $PostgreSQL: pgsql/src/port/exec.c,v 1.59 2008/03/31 01:31:43 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -80,8 +80,8 @@ validate_exec(const char *path)
 #else
 	char		path_exe[MAXPGPATH + sizeof(".exe") - 1];
 #endif
-	int			is_r = 0;
-	int			is_x = 0;
+	int			is_r;
+	int			is_x;
 
 #ifdef WIN32
 	/* Win32 requires a .exe suffix for stat() */
@@ -103,7 +103,7 @@ validate_exec(const char *path)
 	if (stat(path, &buf) < 0)
 		return -1;
 
-	if ((buf.st_mode & S_IFMT) != S_IFREG)
+	if (!S_ISREG(buf.st_mode))
 		return -1;
 
 	/*
@@ -331,7 +331,7 @@ resolve_symlinks(char *path)
 			fname = path;
 
 		if (lstat(fname, &buf) < 0 ||
-			(buf.st_mode & S_IFMT) != S_IFLNK)
+			!S_ISLNK(buf.st_mode))
 			break;
 
 		rllen = readlink(fname, link_buf, sizeof(link_buf));
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
@@ -11,7 +11,7 @@
  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/test/regress/pg_regress.c,v 1.43 2008/03/04 15:38:31 mha Exp $
+ * $PostgreSQL: pgsql/src/test/regress/pg_regress.c,v 1.44 2008/03/31 01:31:43 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1102,7 +1102,7 @@ directory_exists(const char *dir)
 
 	if (stat(dir, &st) != 0)
 		return false;
-	if (st.st_mode & S_IFDIR)
+	if (S_ISDIR(st.st_mode))
 		return true;
 	return false;
 }

Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,7 @@`
`5`	`5`	`* Copyright (c) 2002-2008, PostgreSQL Global Development Group`
`6`	`6`	`*`
`7`	`7`	`* IDENTIFICATION`
`8`		`- * $PostgreSQL: pgsql/src/backend/utils/adt/dbsize.c,v 1.17 2008/03/25 22:42:44 tgl Exp $`
	`8`	`+ * $PostgreSQL: pgsql/src/backend/utils/adt/dbsize.c,v 1.18 2008/03/31 01:31:43 tgl Exp $`
`9`	`9`	`*`
`10`	`10`	`*/`
`11`	`11`
`@@ -209,7 +209,7 @@ calculate_tablespace_size(Oid tblspcOid)`
`209`	`209`	`errmsg("could not stat file \"%s\": %m", pathname)));`
`210`	`210`	`}`
`211`	`211`
`212`		`- if (fst.st_mode & S_IFDIR)`
	`212`	`+ if (S_ISDIR(fst.st_mode))`
`213`	`213`	`totalsize += db_dir_size(pathname);`
`214`	`214`
`215`	`215`	`totalsize += fst.st_size;`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@`
`11`	`11`	`* as a service.`
`12`	`12`	`*`
`13`	`13`	`* IDENTIFICATION`
`14`		`- * $PostgreSQL: pgsql/src/port/copydir.c,v 1.21 2008/01/01 19:46:00 momjian Exp $`
	`14`	`+ * $PostgreSQL: pgsql/src/port/copydir.c,v 1.22 2008/03/31 01:31:43 tgl Exp $`
`15`	`15`	`*`
`16`	`16`	`*-------------------------------------------------------------------------`
`17`	`17`	`*/`
`@@ -80,13 +80,13 @@ copydir(char fromdir, char todir, bool recurse)`
`80`	`80`	`(errcode_for_file_access(),`
`81`	`81`	`errmsg("could not stat file \"%s\": %m", fromfile)));`
`82`	`82`
`83`		`- if (fst.st_mode & S_IFDIR)`
	`83`	`+ if (S_ISDIR(fst.st_mode))`
`84`	`84`	`{`
`85`	`85`	`/* recurse to handle subdirectories */`
`86`	`86`	`if (recurse)`
`87`	`87`	`copydir(fromfile, tofile, true);`
`88`	`88`	`}`
`89`		`- else if (fst.st_mode & S_IFREG)`
	`89`	`+ else if (S_ISREG(fst.st_mode))`
`90`	`90`	`copy_file(fromfile, tofile);`
`91`	`91`	`}`
`92`	`92`