Il 02/02/15 21:48, Robert Haas ha scritto: > On Sat, Jan 31, 2015 at 8:28 AM, Marco Nenciarini > <marco.nenciar...@2ndquadrant.it> wrote: >> Il 30/01/15 03:54, Michael Paquier ha scritto: >>> On Fri, Jan 30, 2015 at 2:49 AM, Tom Lane <t...@sss.pgh.pa.us> 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 marco.nenciar...@2ndquadrant.it | 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