Re: pg_archivecleanup bug

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Kevin Grittner <kgrittn(at)ymail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_archivecleanup bug
Date: 2014-03-13 05:48:58
Message-ID: 20140313054858.GA8577@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Dec 9, 2013 at 11:27:28AM -0500, Robert Haas wrote:
> On Thu, Dec 5, 2013 at 6:15 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > But the other usages seem to be in assorted utilities, which
> > will need to do it right for themselves. initdb.c's walkdir() seems to
> > have it right and might be a reasonable model to follow. Or maybe we
> > should invent a frontend-friendly version of ReadDir() rather than
> > duplicating all the error checking code in ten-and-counting places?
>
> If there's enough uniformity in all of those places to make that
> feasible, it certainly seems wise to do it that way. I don't know if
> that's the case, though - e.g. maybe some callers want to exit and
> others do not. pg_resetxlog wants to exit; pg_archivecleanup and
> pg_standby most likely want to print an error and carry on.

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.

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.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

Attachment Content-Type Size
readdir.diff text/x-diff 7.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2014-03-13 06:15:09 Re: jsonb and nested hstore
Previous Message Simon Riggs 2014-03-13 04:47:03 Re: [PATCH] Store Extension Options