On Tue, Mar 18, 2014 at 09:13:28PM +0200, Heikki Linnakangas wrote:
> On 03/18/2014 09:04 PM, Simon Riggs wrote:
> >On 18 March 2014 18:55, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote:
> >
> >>That said, I don't find comma expression to be particularly "not
> >>simple".
> >
> >Maybe, but we've not used it before anywhere in Postgres, so I don't
> >see a reason to start now. Especially since C is not the native
> >language of many people these days and people just won't understand
> >it.
> 
> Agreed. The psqlODBC code is littered with comma expressions, and
> the first time I saw it, it took me a really long time to figure out
> what "if (foo = malloc(...), foo) { } " meant. And I consider myself
> quite experienced with C.

I can see how the comma syntax would be confusing, though it does the
job well.  Attached is a patch that does the double-errno.  However,
some of these loops are large, and there are 'continue' calls in there,
causing the addition of many new errno locations.  I am not totally
comfortable that this new coding layout will stay unbroken.

Would people accept?

        for (errno = 0; (dirent = readdir(dir)) != NULL; errno = 0)

That would keep the errno's together and avoid the 'continue' additions.

-- 
  Bruce Momjian  <br...@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +
diff --git a/contrib/pg_archivecleanup/pg_archivecleanup.c b/contrib/pg_archivecleanup/pg_archivecleanup.c
new file mode 100644
index 7b5484b..405ec48
*** a/contrib/pg_archivecleanup/pg_archivecleanup.c
--- b/contrib/pg_archivecleanup/pg_archivecleanup.c
*************** CleanupPriorWALFiles(void)
*** 106,111 ****
--- 106,112 ----
  
  	if ((xldir = opendir(archiveLocation)) != NULL)
  	{
+ 		errno = 0;
  		while ((xlde = readdir(xldir)) != NULL)
  		{
  			strncpy(walfile, xlde->d_name, MAXPGPATH);
*************** CleanupPriorWALFiles(void)
*** 148,153 ****
--- 149,155 ----
  						fprintf(stderr,
  								"%s: file \"%s\" would be removed\n",
  								progname, WALFilePath);
+ 					errno = 0;
  					continue;
  				}
  
*************** CleanupPriorWALFiles(void)
*** 163,170 ****
  					break;
  				}
  			}
  		}
! 		closedir(xldir);
  	}
  	else
  		fprintf(stderr, "%s: could not open archive location \"%s\": %s\n",
--- 165,185 ----
  					break;
  				}
  			}
+ 			errno = 0;
  		}
! 
! #ifdef WIN32
! 		/* Bug in old Mingw dirent.c;  fixed in mingw-runtime-3.2, 2003-10-10 */
! 		if (GetLastError() == ERROR_NO_MORE_FILES)
! 			errno = 0;
! #endif
! 
! 		if (errno)
! 			fprintf(stderr, "%s: could not read archive location \"%s\": %s\n",
! 					progname, archiveLocation, strerror(errno));
! 		if (!closedir(xldir))
! 			fprintf(stderr, "%s: could not close archive location \"%s\": %s\n",
! 					progname, archiveLocation, strerror(errno));
  	}
  	else
  		fprintf(stderr, "%s: could not open archive location \"%s\": %s\n",
diff --git a/contrib/pg_standby/pg_standby.c b/contrib/pg_standby/pg_standby.c
new file mode 100644
index 8ddd486..d4731d7
*** a/contrib/pg_standby/pg_standby.c
--- b/contrib/pg_standby/pg_standby.c
*************** CustomizableCleanupPriorWALFiles(void)
*** 245,250 ****
--- 245,251 ----
  		 */
  		if ((xldir = opendir(archiveLocation)) != NULL)
  		{
+ 			errno = 0;
  			while ((xlde = readdir(xldir)) != NULL)
  			{
  				/*
*************** CustomizableCleanupPriorWALFiles(void)
*** 282,288 ****
--- 283,300 ----
  						break;
  					}
  				}
+ 				errno = 0;
  			}
+ 
+ #ifdef WIN32
+ 			/* Bug in old Mingw dirent.c;  fixed in mingw-runtime-3.2, 2003-10-10 */
+ 			if (GetLastError() == ERROR_NO_MORE_FILES)
+ 				errno = 0;
+ #endif
+ 
+ 			if (errno)
+ 				fprintf(stderr, "%s: could not read archive location \"%s\": %s\n",
+ 						progname, archiveLocation, strerror(errno));
  			if (debug)
  				fprintf(stderr, "\n");
  		}
*************** CustomizableCleanupPriorWALFiles(void)
*** 290,296 ****
  			fprintf(stderr, "%s: could not open archive location \"%s\": %s\n",
  					progname, archiveLocation, strerror(errno));
  
! 		closedir(xldir);
  		fflush(stderr);
  	}
  }
--- 302,311 ----
  			fprintf(stderr, "%s: could not open archive location \"%s\": %s\n",
  					progname, archiveLocation, strerror(errno));
  
! 		if (!closedir(xldir))
! 			fprintf(stderr, "%s: could not close archive location \"%s\": %s\n",
! 					progname, archiveLocation, strerror(errno));
! 		
  		fflush(stderr);
  	}
  }
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
new file mode 100644
index 4dc809d..5158cfe
*** a/src/backend/storage/file/fd.c
--- b/src/backend/storage/file/fd.c
*************** ReadDir(DIR *dir, const char *dirname)
*** 1957,1966 ****
  		return dent;
  
  #ifdef WIN32
! 	/*
! 	 * This fix is in mingw cvs (runtime/mingwex/dirent.c rev 1.4), but not in
! 	 * released version
! 	 */
  	if (GetLastError() == ERROR_NO_MORE_FILES)
  		errno = 0;
  #endif
--- 1957,1963 ----
  		return dent;
  
  #ifdef WIN32
! 	/* Bug in old Mingw dirent.c;  fixed in mingw-runtime-3.2, 2003-10-10 */
  	if (GetLastError() == ERROR_NO_MORE_FILES)
  		errno = 0;
  #endif
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
new file mode 100644
index 61bd785..7416377
*** a/src/bin/initdb/initdb.c
--- b/src/bin/initdb/initdb.c
*************** walkdir(char *path, void (*action) (char
*** 541,553 ****
  		exit_nicely();
  	}
  
! 	while (errno = 0, (direntry = readdir(dir)) != NULL)
  	{
  		struct stat fst;
  
  		if (strcmp(direntry->d_name, ".") == 0 ||
  			strcmp(direntry->d_name, "..") == 0)
  			continue;
  
  		snprintf(subpath, MAXPGPATH, "%s/%s", path, direntry->d_name);
  
--- 541,557 ----
  		exit_nicely();
  	}
  
! 	errno = 0;
! 	while ((direntry = readdir(dir)) != NULL)
  	{
  		struct stat fst;
  
  		if (strcmp(direntry->d_name, ".") == 0 ||
  			strcmp(direntry->d_name, "..") == 0)
+ 		{
+ 			errno = 0;
  			continue;
+ 		}
  
  		snprintf(subpath, MAXPGPATH, "%s/%s", path, direntry->d_name);
  
*************** walkdir(char *path, void (*action) (char
*** 562,574 ****
  			walkdir(subpath, action);
  		else if (S_ISREG(fst.st_mode))
  			(*action) (subpath, false);
  	}
  
  #ifdef WIN32
! 	/*
! 	 * This fix is in mingw cvs (runtime/mingwex/dirent.c rev 1.4), but not in
! 	 * released version
! 	 */
  	if (GetLastError() == ERROR_NO_MORE_FILES)
  		errno = 0;
  #endif
--- 566,576 ----
  			walkdir(subpath, action);
  		else if (S_ISREG(fst.st_mode))
  			(*action) (subpath, false);
+ 		errno = 0;
  	}
  
  #ifdef WIN32
! 	/* Bug in old Mingw dirent.c;  fixed in mingw-runtime-3.2, 2003-10-10 */
  	if (GetLastError() == ERROR_NO_MORE_FILES)
  		errno = 0;
  #endif
*************** walkdir(char *path, void (*action) (char
*** 580,586 ****
  		exit_nicely();
  	}
  
! 	closedir(dir);
  
  	/*
  	 * It's important to fsync the destination directory itself as individual
--- 582,593 ----
  		exit_nicely();
  	}
  
! 	if (!closedir(dir))
! 	{
! 		fprintf(stderr, _("%s: could not close directory \"%s\": %s\n"),
! 				progname, path, strerror(errno));
! 		exit_nicely();
! 	}
  
  	/*
  	 * It's important to fsync the destination directory itself as individual
diff --git a/src/bin/pg_basebackup/pg_receivexlog.c b/src/bin/pg_basebackup/pg_receivexlog.c
new file mode 100644
index 2478789..9b62edf
*** a/src/bin/pg_basebackup/pg_receivexlog.c
--- b/src/bin/pg_basebackup/pg_receivexlog.c
*************** FindStreamingStart(uint32 *tli)
*** 139,144 ****
--- 139,145 ----
  		disconnect_and_exit(1);
  	}
  
+ 	errno = 0;
  	while ((dirent = readdir(dir)) != NULL)
  	{
  		uint32		tli;
*************** FindStreamingStart(uint32 *tli)
*** 153,171 ****
--- 154,184 ----
  		if (strlen(dirent->d_name) == 24)
  		{
  			if (strspn(dirent->d_name, "0123456789ABCDEF") != 24)
+ 			{
+ 				errno = 0;
  				continue;
+ 			}
  			ispartial = false;
  		}
  		else if (strlen(dirent->d_name) == 32)
  		{
  			if (strspn(dirent->d_name, "0123456789ABCDEF") != 24)
+ 			{
+ 				errno = 0;
  				continue;
+ 			}
  			if (strcmp(&dirent->d_name[24], ".partial") != 0)
+ 			{
+ 				errno = 0;
  				continue;
+ 			}
  			ispartial = true;
  		}
  		else
+ 		{
+ 			errno = 0;
  			continue;
+ 		}
  
  		/*
  		 * Looks like an xlog file. Parse its position.
*************** FindStreamingStart(uint32 *tli)
*** 194,199 ****
--- 207,213 ----
  				fprintf(stderr,
  						_("%s: segment file \"%s\" has incorrect size %d, skipping\n"),
  						progname, dirent->d_name, (int) statbuf.st_size);
+ 				errno = 0;
  				continue;
  			}
  		}
*************** FindStreamingStart(uint32 *tli)
*** 207,215 ****
  			high_tli = tli;
  			high_ispartial = ispartial;
  		}
  	}
  
! 	closedir(dir);
  
  	if (high_segno > 0)
  	{
--- 221,248 ----
  			high_tli = tli;
  			high_ispartial = ispartial;
  		}
+ 		errno = 0;
  	}
  
! #ifdef WIN32
! 	/* Bug in old Mingw dirent.c;  fixed in mingw-runtime-3.2, 2003-10-10 */
! 	if (GetLastError() == ERROR_NO_MORE_FILES)
! 		errno = 0;
! #endif
! 
! 	if (errno)
! 	{
! 		fprintf(stderr, _("%s: could not read directory \"%s\": %s\n"),
! 				progname, basedir, strerror(errno));
! 		disconnect_and_exit(1);
! 	}
! 
! 	if (!closedir(dir))
! 	{
! 		fprintf(stderr, _("%s: could not close directory \"%s\": %s\n"),
! 				progname, basedir, strerror(errno));
! 		disconnect_and_exit(1);
! 	}
  
  	if (high_segno > 0)
  	{
diff --git a/src/bin/pg_dump/pg_backup_directory.c b/src/bin/pg_dump/pg_backup_directory.c
new file mode 100644
index 1bed8a9..161bfb0
*** a/src/bin/pg_dump/pg_backup_directory.c
--- b/src/bin/pg_dump/pg_backup_directory.c
*************** InitArchiveFmt_Directory(ArchiveHandle *
*** 177,182 ****
--- 177,183 ----
  				struct dirent *d;
  
  				is_empty = true;
+ 				errno = 0;
  				while ((d = readdir(dir)))
  				{
  					if (strcmp(d->d_name, ".") != 0 && strcmp(d->d_name, "..") != 0)
*************** InitArchiveFmt_Directory(ArchiveHandle *
*** 184,191 ****
  						is_empty = false;
  						break;
  					}
  				}
! 				closedir(dir);
  			}
  		}
  
--- 185,205 ----
  						is_empty = false;
  						break;
  					}
+ 					errno = 0;
  				}
! 
! #ifdef WIN32
! 				/* Bug in old Mingw dirent.c;  fixed in mingw-runtime-3.2, 2003-10-10 */
! 				if (GetLastError() == ERROR_NO_MORE_FILES)
! 					errno = 0;
! #endif
! 
! 				if (errno)
! 					exit_horribly(modulename, "could not read directory \"%s\": %s\n",
! 								  ctx->directory, strerror(errno));
! 				if (!closedir(dir))
! 					exit_horribly(modulename, "could not close directory \"%s\": %s\n",
! 								  ctx->directory, strerror(errno));
  			}
  		}
  
diff --git a/src/bin/pg_resetxlog/pg_resetxlog.c b/src/bin/pg_resetxlog/pg_resetxlog.c
new file mode 100644
index 28a4f19..e63a8e4
*** a/src/bin/pg_resetxlog/pg_resetxlog.c
--- b/src/bin/pg_resetxlog/pg_resetxlog.c
*************** FindEndOfXLOG(void)
*** 848,868 ****
  	}
  
  #ifdef WIN32
! 	/*
! 	 * This fix is in mingw cvs (runtime/mingwex/dirent.c rev 1.4), but not in
! 	 * released version
! 	 */
  	if (GetLastError() == ERROR_NO_MORE_FILES)
  		errno = 0;
  #endif
  
  	if (errno)
  	{
! 		fprintf(stderr, _("%s: could not read from directory \"%s\": %s\n"),
  				progname, XLOGDIR, strerror(errno));
  		exit(1);
  	}
- 	closedir(xldir);
  
  	/*
  	 * Finally, convert to new xlog seg size, and advance by one to ensure we
--- 848,870 ----
  	}
  
  #ifdef WIN32
! 	/* Bug in old Mingw dirent.c;  fixed in mingw-runtime-3.2, 2003-10-10 */
  	if (GetLastError() == ERROR_NO_MORE_FILES)
  		errno = 0;
  #endif
  
  	if (errno)
  	{
! 		fprintf(stderr, _("%s: could not read directory \"%s\": %s\n"),
! 				progname, XLOGDIR, strerror(errno));
! 		exit(1);
! 	}
! 	if (!closedir(xldir))
! 	{
! 		fprintf(stderr, _("%s: could not close directory \"%s\": %s\n"),
  				progname, XLOGDIR, strerror(errno));
  		exit(1);
  	}
  
  	/*
  	 * Finally, convert to new xlog seg size, and advance by one to ensure we
*************** KillExistingXLOG(void)
*** 910,930 ****
  	}
  
  #ifdef WIN32
! 	/*
! 	 * This fix is in mingw cvs (runtime/mingwex/dirent.c rev 1.4), but not in
! 	 * released version
! 	 */
  	if (GetLastError() == ERROR_NO_MORE_FILES)
  		errno = 0;
  #endif
  
  	if (errno)
  	{
! 		fprintf(stderr, _("%s: could not read from directory \"%s\": %s\n"),
  				progname, XLOGDIR, strerror(errno));
  		exit(1);
  	}
- 	closedir(xldir);
  }
  
  
--- 912,934 ----
  	}
  
  #ifdef WIN32
! 	/* Bug in old Mingw dirent.c;  fixed in mingw-runtime-3.2, 2003-10-10 */
  	if (GetLastError() == ERROR_NO_MORE_FILES)
  		errno = 0;
  #endif
  
  	if (errno)
  	{
! 		fprintf(stderr, _("%s: could not read directory \"%s\": %s\n"),
! 				progname, XLOGDIR, strerror(errno));
! 		exit(1);
! 	}
! 	if (!closedir(xldir))
! 	{
! 		fprintf(stderr, _("%s: could not close directory \"%s\": %s\n"),
  				progname, XLOGDIR, strerror(errno));
  		exit(1);
  	}
  }
  
  
*************** KillExistingArchiveStatus(void)
*** 967,987 ****
  	}
  
  #ifdef WIN32
! 	/*
! 	 * This fix is in mingw cvs (runtime/mingwex/dirent.c rev 1.4), but not in
! 	 * released version
! 	 */
  	if (GetLastError() == ERROR_NO_MORE_FILES)
  		errno = 0;
  #endif
  
  	if (errno)
  	{
! 		fprintf(stderr, _("%s: could not read from directory \"%s\": %s\n"),
  				progname, ARCHSTATDIR, strerror(errno));
  		exit(1);
  	}
- 	closedir(xldir);
  }
  
  
--- 971,993 ----
  	}
  
  #ifdef WIN32
! 	/* Bug in old Mingw dirent.c;  fixed in mingw-runtime-3.2, 2003-10-10 */
  	if (GetLastError() == ERROR_NO_MORE_FILES)
  		errno = 0;
  #endif
  
  	if (errno)
  	{
! 		fprintf(stderr, _("%s: could not read directory \"%s\": %s\n"),
! 				progname, ARCHSTATDIR, strerror(errno));
! 		exit(1);
! 	}
! 	if (!closedir(xldir))
! 	{
! 		fprintf(stderr, _("%s: could not close directory \"%s\": %s\n"),
  				progname, ARCHSTATDIR, strerror(errno));
  		exit(1);
  	}
  }
  
  
diff --git a/src/common/pgfnames.c b/src/common/pgfnames.c
new file mode 100644
index 9a68163..411acfd
*** a/src/common/pgfnames.c
--- b/src/common/pgfnames.c
*************** pgfnames(const char *path)
*** 67,76 ****
  	}
  
  #ifdef WIN32
! 	/*
! 	 * This fix is in mingw cvs (runtime/mingwex/dirent.c rev 1.4), but not in
! 	 * released version
! 	 */
  	if (GetLastError() == ERROR_NO_MORE_FILES)
  		errno = 0;
  #endif
--- 67,73 ----
  	}
  
  #ifdef WIN32
! 	/* Bug in old Mingw dirent.c;  fixed in mingw-runtime-3.2, 2003-10-10 */
  	if (GetLastError() == ERROR_NO_MORE_FILES)
  		errno = 0;
  #endif
*************** pgfnames(const char *path)
*** 87,93 ****
  
  	filenames[numnames] = NULL;
  
! 	closedir(dir);
  
  	return filenames;
  }
--- 84,98 ----
  
  	filenames[numnames] = NULL;
  
! 	if (!closedir(dir))
! 	{
! #ifndef FRONTEND
! 		elog(WARNING, "could not close directory \"%s\": %m", path);
! #else
! 		fprintf(stderr, _("could not close directory \"%s\": %s\n"),
! 				path, strerror(errno));
! #endif
! 	}
  
  	return filenames;
  }
diff --git a/src/port/dirent.c b/src/port/dirent.c
new file mode 100644
index f9d93ea..31121c2
*** a/src/port/dirent.c
--- b/src/port/dirent.c
*************** readdir(DIR *d)
*** 111,119 ****
  int
  closedir(DIR *d)
  {
  	if (d->handle != INVALID_HANDLE_VALUE)
! 		FindClose(d->handle);
  	free(d->dirname);
  	free(d);
! 	return 0;
  }
--- 111,121 ----
  int
  closedir(DIR *d)
  {
+ 	int ret = 1;
+ 	
  	if (d->handle != INVALID_HANDLE_VALUE)
! 		ret = FindClose(d->handle);
  	free(d->dirname);
  	free(d);
! 	return !ret;
  }
diff --git a/src/port/pgcheckdir.c b/src/port/pgcheckdir.c
new file mode 100644
index fc97f8c..70d5cd1
*** a/src/port/pgcheckdir.c
--- b/src/port/pgcheckdir.c
*************** pg_check_dir(const char *dir)
*** 33,51 ****
  	struct dirent *file;
  	bool		dot_found = false;
  
- 	errno = 0;
- 
  	chkdir = opendir(dir);
- 
  	if (chkdir == NULL)
  		return (errno == ENOENT) ? 0 : -1;
  
  	while ((file = readdir(chkdir)) != NULL)
  	{
  		if (strcmp(".", file->d_name) == 0 ||
  			strcmp("..", file->d_name) == 0)
  		{
  			/* skip this and parent directory */
  			continue;
  		}
  #ifndef WIN32
--- 33,50 ----
  	struct dirent *file;
  	bool		dot_found = false;
  
  	chkdir = opendir(dir);
  	if (chkdir == NULL)
  		return (errno == ENOENT) ? 0 : -1;
  
+ 	errno = 0;
  	while ((file = readdir(chkdir)) != NULL)
  	{
  		if (strcmp(".", file->d_name) == 0 ||
  			strcmp("..", file->d_name) == 0)
  		{
  			/* skip this and parent directory */
+ 			errno = 0;
  			continue;
  		}
  #ifndef WIN32
*************** pg_check_dir(const char *dir)
*** 65,84 ****
  			result = 4;			/* not empty */
  			break;
  		}
  	}
  
  #ifdef WIN32
! 	/*
! 	 * This fix is in mingw cvs (runtime/mingwex/dirent.c rev 1.4), but not in
! 	 * released version
! 	 */
  	if (GetLastError() == ERROR_NO_MORE_FILES)
  		errno = 0;
  #endif
  
! 	closedir(chkdir);
! 
! 	if (errno != 0)
  		result = -1;			/* some kind of I/O error? */
  
  	/* We report on dot-files if we _only_ find dot files */
--- 64,79 ----
  			result = 4;			/* not empty */
  			break;
  		}
+ 		errno = 0;
  	}
  
  #ifdef WIN32
! 	/* Bug in old Mingw dirent.c;  fixed in mingw-runtime-3.2, 2003-10-10 */
  	if (GetLastError() == ERROR_NO_MORE_FILES)
  		errno = 0;
  #endif
  
! 	if (errno || closedir(chkdir) == -1)
  		result = -1;			/* some kind of I/O error? */
  
  	/* We report on dot-files if we _only_ find dot files */
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to