Re: pg_archivecleanup bug - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: pg_archivecleanup bug
Date
Msg-id CAA4eK1+un6DpWxLCyx__PTKJ1yS8BYR4EoF+GXkcXg_4JN5JVA@mail.gmail.com
Whole thread Raw
In response to Re: pg_archivecleanup bug  (Bruce Momjian <bruce@momjian.us>)
Responses Re: pg_archivecleanup bug
List pgsql-hackers
On Thu, Mar 13, 2014 at 11:18 AM, Bruce Momjian <bruce@momjian.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



pgsql-hackers by date:

Previous
From: Atri Sharma
Date:
Subject: Re: Planner hints in Postgresql
Next
From: Jeff Janes
Date:
Subject: Re: Planner hints in Postgresql