On Wed, Mar 19, 2014 at 09:59:19AM +0200, Heikki Linnakangas wrote:
> >Would people accept?
> >
> >     for (errno = 0; (dirent = readdir(dir)) != NULL; errno = 0)
> >
> >That would keep the errno's together and avoid the 'continue' additions.
> 
> That's clever. A less clever way would be:
> 
> for (;;)
> {
>   errno = 0;
>   if ((dirent = readdir(dir)) != NULL)
>     break;
> 
>   ...
> }
> 
> I'm fine with either, but that's how I would naturally write it.
> 
> Yet another way would be to have a wrapper function for readdir that
> resets errno, and just replace the current readdir() calls with
> that.
> 
> And now that I look at initdb.c, walkdir is using the comma
> expression for this already. So there's some precedence, and it
> doesn't actually look that bad. So I withdraw my objection for that
> approach; I'm fine with any of the discussed alternatives, really.

OK, let's go with the comma.  Ironically, the for() loop would be an odd
way to avoid commas as 'for' uses commas often:

        for (i = 0, j = 1; i < 10; i++, j++)

The attached patch is slightly updated.  I will apply it to head and all
the back branches, including the stylistic change to pg_resetxlog (for
consistency) and remove the MinGW block in head.

-- 
  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..4f4d507
*** a/contrib/pg_archivecleanup/pg_archivecleanup.c
--- b/contrib/pg_archivecleanup/pg_archivecleanup.c
*************** CleanupPriorWALFiles(void)
*** 106,112 ****
  
  	if ((xldir = opendir(archiveLocation)) != NULL)
  	{
! 		while ((xlde = readdir(xldir)) != NULL)
  		{
  			strncpy(walfile, xlde->d_name, MAXPGPATH);
  			TrimExtension(walfile, additional_ext);
--- 106,112 ----
  
  	if ((xldir = opendir(archiveLocation)) != NULL)
  	{
! 		while (errno = 0, (xlde = readdir(xldir)) != NULL)
  		{
  			strncpy(walfile, xlde->d_name, MAXPGPATH);
  			TrimExtension(walfile, additional_ext);
*************** CleanupPriorWALFiles(void)
*** 164,170 ****
  				}
  			}
  		}
! 		closedir(xldir);
  	}
  	else
  		fprintf(stderr, "%s: could not open archive location \"%s\": %s\n",
--- 164,182 ----
  				}
  			}
  		}
! 
! #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..70fb387
*** a/contrib/pg_standby/pg_standby.c
--- b/contrib/pg_standby/pg_standby.c
*************** CustomizableCleanupPriorWALFiles(void)
*** 245,251 ****
  		 */
  		if ((xldir = opendir(archiveLocation)) != NULL)
  		{
! 			while ((xlde = readdir(xldir)) != NULL)
  			{
  				/*
  				 * We ignore the timeline part of the XLOG segment identifiers
--- 245,251 ----
  		 */
  		if ((xldir = opendir(archiveLocation)) != NULL)
  		{
! 			while (errno = 0, (xlde = readdir(xldir)) != NULL)
  			{
  				/*
  				 * We ignore the timeline part of the XLOG segment identifiers
*************** CustomizableCleanupPriorWALFiles(void)
*** 283,288 ****
--- 283,298 ----
  					}
  				}
  			}
+ 
+ #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);
  	}
  }
--- 300,309 ----
  			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..f23d988
*** a/src/bin/initdb/initdb.c
--- b/src/bin/initdb/initdb.c
*************** walkdir(char *path, void (*action) (char
*** 565,574 ****
  	}
  
  #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
--- 565,571 ----
  	}
  
  #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
--- 577,588 ----
  		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..b742f0c
*** a/src/bin/pg_basebackup/pg_receivexlog.c
--- b/src/bin/pg_basebackup/pg_receivexlog.c
*************** FindStreamingStart(uint32 *tli)
*** 139,145 ****
  		disconnect_and_exit(1);
  	}
  
! 	while ((dirent = readdir(dir)) != NULL)
  	{
  		uint32		tli;
  		XLogSegNo	segno;
--- 139,145 ----
  		disconnect_and_exit(1);
  	}
  
! 	while (errno = 0, (dirent = readdir(dir)) != NULL)
  	{
  		uint32		tli;
  		XLogSegNo	segno;
*************** FindStreamingStart(uint32 *tli)
*** 209,215 ****
  		}
  	}
  
! 	closedir(dir);
  
  	if (high_segno > 0)
  	{
--- 209,233 ----
  		}
  	}
  
! #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..c450cda
*** a/src/bin/pg_dump/pg_backup_directory.c
--- b/src/bin/pg_dump/pg_backup_directory.c
*************** InitArchiveFmt_Directory(ArchiveHandle *
*** 177,183 ****
  				struct dirent *d;
  
  				is_empty = true;
! 				while ((d = readdir(dir)))
  				{
  					if (strcmp(d->d_name, ".") != 0 && strcmp(d->d_name, "..") != 0)
  					{
--- 177,183 ----
  				struct dirent *d;
  
  				is_empty = true;
! 				while (errno = 0, (d = readdir(dir)))
  				{
  					if (strcmp(d->d_name, ".") != 0 && strcmp(d->d_name, "..") != 0)
  					{
*************** InitArchiveFmt_Directory(ArchiveHandle *
*** 185,191 ****
  						break;
  					}
  				}
! 				closedir(dir);
  			}
  		}
  
--- 185,204 ----
  						break;
  					}
  				}
! 
! #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..b6cd72b
*** a/src/bin/pg_resetxlog/pg_resetxlog.c
--- b/src/bin/pg_resetxlog/pg_resetxlog.c
*************** FindEndOfXLOG(void)
*** 821,828 ****
  		exit(1);
  	}
  
! 	errno = 0;
! 	while ((xlde = readdir(xldir)) != NULL)
  	{
  		if (strlen(xlde->d_name) == 24 &&
  			strspn(xlde->d_name, "0123456789ABCDEF") == 24)
--- 821,827 ----
  		exit(1);
  	}
  
! 	while (errno = 0, (xlde = readdir(xldir)) != NULL)
  	{
  		if (strlen(xlde->d_name) == 24 &&
  			strspn(xlde->d_name, "0123456789ABCDEF") == 24)
*************** FindEndOfXLOG(void)
*** 844,868 ****
  			if (segno > newXlogSegNo)
  				newXlogSegNo = segno;
  		}
- 		errno = 0;
  	}
  
  #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
--- 843,869 ----
  			if (segno > newXlogSegNo)
  				newXlogSegNo = segno;
  		}
  	}
  
  #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)
*** 892,899 ****
  		exit(1);
  	}
  
! 	errno = 0;
! 	while ((xlde = readdir(xldir)) != NULL)
  	{
  		if (strlen(xlde->d_name) == 24 &&
  			strspn(xlde->d_name, "0123456789ABCDEF") == 24)
--- 893,899 ----
  		exit(1);
  	}
  
! 	while (errno = 0, (xlde = readdir(xldir)) != NULL)
  	{
  		if (strlen(xlde->d_name) == 24 &&
  			strspn(xlde->d_name, "0123456789ABCDEF") == 24)
*************** KillExistingXLOG(void)
*** 906,930 ****
  				exit(1);
  			}
  		}
- 		errno = 0;
  	}
  
  #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);
  }
  
  
--- 906,932 ----
  				exit(1);
  			}
  		}
  	}
  
  #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)
*** 948,955 ****
  		exit(1);
  	}
  
! 	errno = 0;
! 	while ((xlde = readdir(xldir)) != NULL)
  	{
  		if (strspn(xlde->d_name, "0123456789ABCDEF") == 24 &&
  			(strcmp(xlde->d_name + 24, ".ready") == 0 ||
--- 950,956 ----
  		exit(1);
  	}
  
! 	while (errno = 0, (xlde = readdir(xldir)) != NULL)
  	{
  		if (strspn(xlde->d_name, "0123456789ABCDEF") == 24 &&
  			(strcmp(xlde->d_name + 24, ".ready") == 0 ||
*************** KillExistingArchiveStatus(void)
*** 963,987 ****
  				exit(1);
  			}
  		}
- 		errno = 0;
  	}
  
  #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);
  }
  
  
--- 964,990 ----
  				exit(1);
  			}
  		}
  	}
  
  #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..4497272
*** a/src/common/pgfnames.c
--- b/src/common/pgfnames.c
*************** pgfnames(const char *path)
*** 50,57 ****
  
  	filenames = (char **) palloc(fnsize * sizeof(char *));
  
! 	errno = 0;
! 	while ((file = readdir(dir)) != NULL)
  	{
  		if (strcmp(file->d_name, ".") != 0 && strcmp(file->d_name, "..") != 0)
  		{
--- 50,56 ----
  
  	filenames = (char **) palloc(fnsize * sizeof(char *));
  
! 	while (errno = 0, (file = readdir(dir)) != NULL)
  	{
  		if (strcmp(file->d_name, ".") != 0 && strcmp(file->d_name, "..") != 0)
  		{
*************** pgfnames(const char *path)
*** 63,76 ****
  			}
  			filenames[numnames++] = pstrdup(file->d_name);
  		}
- 		errno = 0;
  	}
  
  #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
--- 62,71 ----
  			}
  			filenames[numnames++] = pstrdup(file->d_name);
  		}
  	}
  
  #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;
  }
--- 82,96 ----
  
  	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..ed5661c
*** 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 = 0;
+ 	
  	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..f10a84c
*** a/src/port/pgcheckdir.c
--- b/src/port/pgcheckdir.c
*************** pg_check_dir(const char *dir)
*** 33,46 ****
  	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)
--- 33,43 ----
  	struct dirent *file;
  	bool		dot_found = false;
  
  	chkdir = opendir(dir);
  	if (chkdir == NULL)
  		return (errno == ENOENT) ? 0 : -1;
  
! 	while (errno = 0, (file = readdir(chkdir)) != NULL)
  	{
  		if (strcmp(".", file->d_name) == 0 ||
  			strcmp("..", file->d_name) == 0)
*************** pg_check_dir(const char *dir)
*** 68,84 ****
  	}
  
  #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 */
--- 65,76 ----
  	}
  
  #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