Fix error handling of readdir() port implementation on first file lookup

michaelpq · michaelpq · commit 81f5b32839ff · 2019-03-04T09:50:24.000+09:00
The implementation of readdir() in src/port/ which gets used by MSVC has been added in 399a36a, and since the beginning it considers all errors on the first file lookup as ENOENT, setting errno accordingly and letting the routine caller think that the directory is empty. While this is normally enough for the case of the backend, this can confuse callers of this routine on Windows as all errors would map to the same behavior. So, for example, even permission errors would be thought as having an empty directory, while there could be contents in it. This commit changes the error handling so as readdir() gets a behavior similar to native implementations: force errno=0 when seeing ERROR_FILE_NOT_FOUND as error and consider other errors as plain failures. While looking at the patch, I noticed that MinGW does not enforce errno=0 when looking at the first file, but it gets enforced on the next file lookups. A comment related to that was incorrect in the code. Reported-by: Yuri Kurenkov Diagnosed-by: Yuri Kurenkov, Grigory Smolkin Author: Konstantin Knizhnik Reviewed-by: Andrew Dunstan, Michael Paquier Discussion: https://postgr.es/m/2cad7829-8d66-e39c-b937-ac825db5203d@postgrespro.ru Backpatch-through: 9.4
diff --git a/src/port/dirent.c b/src/port/dirent.c
@@ -84,21 +84,23 @@ readdir(DIR *d)
 		d->handle = FindFirstFile(d->dirname, &fd);
 		if (d->handle == INVALID_HANDLE_VALUE)
 		{
-			errno = ENOENT;
+			/* If there are no files, force errno=0 (unlike mingw) */
+			if (GetLastError() == ERROR_FILE_NOT_FOUND)
+				errno = 0;
+			else
+				_dosmaperr(GetLastError());
 			return NULL;
 		}
 	}
 	else
 	{
 		if (!FindNextFile(d->handle, &fd))
 		{
+			/* If there are no more files, force errno=0 (like mingw) */
 			if (GetLastError() == ERROR_NO_MORE_FILES)
-			{
-				/* No more files, force errno=0 (unlike mingw) */
 				errno = 0;
-				return NULL;
-			}
-			_dosmaperr(GetLastError());
+			else
+				_dosmaperr(GetLastError());
 			return NULL;
 		}
 	}

Original file line number	Diff line number	Diff line change
`@@ -84,21 +84,23 @@ readdir(DIR *d)`
`84`	`84`	`d->handle = FindFirstFile(d->dirname, &fd);`
`85`	`85`	`if (d->handle == INVALID_HANDLE_VALUE)`
`86`	`86`	`{`
`87`		`- errno = ENOENT;`
	`87`	`+ /* If there are no files, force errno=0 (unlike mingw) */`
	`88`	`+ if (GetLastError() == ERROR_FILE_NOT_FOUND)`
	`89`	`+ errno = 0;`
	`90`	`+ else`
	`91`	`+ _dosmaperr(GetLastError());`
`88`	`92`	`return NULL;`
`89`	`93`	`}`
`90`	`94`	`}`
`91`	`95`	`else`
`92`	`96`	`{`
`93`	`97`	`if (!FindNextFile(d->handle, &fd))`
`94`	`98`	`{`
	`99`	`+ /* If there are no more files, force errno=0 (like mingw) */`
`95`	`100`	`if (GetLastError() == ERROR_NO_MORE_FILES)`
`96`		`- {`
`97`		`- /* No more files, force errno=0 (unlike mingw) */`
`98`	`101`	`errno = 0;`
`99`		`- return NULL;`
`100`		`- }`
`101`		`- _dosmaperr(GetLastError());`
	`102`	`+ else`
	`103`	`+ _dosmaperr(GetLastError());`
`102`	`104`	`return NULL;`
`103`	`105`	`}`
`104`	`106`	`}`