Re: pg_archivecleanup bug

On Thu, Mar 13, 2014 at 11:18 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
> I have developed the attached patch which fixes all cases where
> readdir() wasn't checking for errno, and cleaned up the syntax in other
> cases to be consistent.

1. One common thing missed wherever handling for errno is added
is below check which is present in all existing cases where errno
is used (initdb.c, pg_resetxlog.c, ReadDir, ..)

#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

2.
! if (errno || closedir(chkdir) == -1)
result = -1; /* some kind of I/O error? */

Is there a special need to check return value of closedir in this
function, as all other uses (initdb.c, pg_resetxlog.c, pgfnames.c)
of it in similar context doesn't check the same?

One thing I think for which this code needs change is to check
errno before closedir as is done in initdb.c or pg_resetxlog.c

> While I am not a fan of backpatching, the fact we are ignoring errors in
> some critical cases seems the non-cosmetic parts should be backpatched.

+1

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com