Il 02/02/15 21:48, Robert Haas ha scritto: > On Sat, Jan 31, 2015 at 8:28 AM, Marco Nenciarini > <[email protected]> wrote: >> Il 30/01/15 03:54, Michael Paquier ha scritto: >>> On Fri, Jan 30, 2015 at 2:49 AM, Tom Lane <[email protected]> wrote: >>>> There is at least one other bug in that function now that I look at it: >>>> in event of a readdir() failure, it neglects to execute closedir(). >>>> Perhaps not too significant since all existing callers will just exit() >>>> anyway after a failure, but still ... >>> I would imagine that code scanners like coverity or similar would not >>> be happy about that. ISTM that it is better to closedir() >>> appropriately in all the code paths. >>> >> >> I've attached a new version of the patch fixing the missing closedir on >> readdir error. > > If readir() fails and closedir() succeeds, the return will be -1 but > errno will be 0. >
The attached patch should fix it. Regards, Marco -- Marco Nenciarini - 2ndQuadrant Italy PostgreSQL Training, Services and Support [email protected] | www.2ndQuadrant.it
diff --git a/src/port/pgcheckdir.c b/src/port/pgcheckdir.c
index 7061893..7102f2c 100644
*** a/src/port/pgcheckdir.c
--- b/src/port/pgcheckdir.c
***************
*** 22,28 ****
* Returns:
* 0 if nonexistent
* 1 if exists and empty
! * 2 if exists and not empty
* -1 if trouble accessing directory (errno reflects the error)
*/
int
--- 22,30 ----
* Returns:
* 0 if nonexistent
* 1 if exists and empty
! * 2 if exists and contains _only_ dot files
! * 3 if exists and contains a mount point
! * 4 if exists and not empty
* -1 if trouble accessing directory (errno reflects the error)
*/
int
*************** pg_check_dir(const char *dir)
*** 32,37 ****
--- 34,41 ----
DIR *chkdir;
struct dirent *file;
bool dot_found = false;
+ bool mount_found = false;
+ int readdir_errno;
chkdir = opendir(dir);
if (chkdir == NULL)
*************** pg_check_dir(const char *dir)
*** 51,60 ****
{
dot_found = true;
}
else if (strcmp("lost+found", file->d_name) == 0)
{
! result = 3; /* not empty, mount
point */
! break;
}
#endif
else
--- 55,64 ----
{
dot_found = true;
}
+ /* lost+found directory */
else if (strcmp("lost+found", file->d_name) == 0)
{
! mount_found = true;
}
#endif
else
*************** pg_check_dir(const char *dir)
*** 64,72 ****
}
}
! if (errno || closedir(chkdir))
result = -1; /* some kind of I/O error? */
/* We report on dot-files if we _only_ find dot files */
if (result == 1 && dot_found)
result = 2;
--- 68,87 ----
}
}
! if (errno)
result = -1; /* some kind of I/O error? */
+ /* Close chkdir and avoid overwriting the readdir errno on success */
+ readdir_errno = errno;
+ if (closedir(chkdir))
+ result = -1; /* error executing closedir */
+ else
+ errno = readdir_errno;
+
+ /* We report on mount point if we find a lost+found directory */
+ if (result == 1 && mount_found)
+ result = 3;
+
/* We report on dot-files if we _only_ find dot files */
if (result == 1 && dot_found)
result = 2;
signature.asc
Description: OpenPGP digital signature
