On Wed, Jul 1, 2015 at 9:53 PM, Fujii Masao <masao.fu...@gmail.com> wrote: > On Wed, Jul 1, 2015 at 8:58 PM, Michael Paquier > <michael.paqu...@gmail.com> wrote: >> On Wed, Jul 1, 2015 at 8:18 PM, Fujii Masao wrote: >>> On Wed, Jul 1, 2015 at 8:16 PM, Fujii Masao wrote: >>>> I updated the patch as follows. Patch attached. >>>> >>>> +#define XLogFileNameExtended(fname, tli, log, seg) >>>> >>>> Move this macro to xlog_internal.h because it's used both in >>>> pg_standby and pg_archivecleanup. There seems no need to >>>> define it independently. >> >> OK for me. >> >>>> -#define MAXFNAMELEN 64 >>>> +#define MAXFNAMELEN 64 >>>> >>>> Revert this unnecessary change. >> >> Yes, thanks. >> >>>> >>>> +/* Length of XLog file name */ >>>> +#define XLOG_DATA_FNAME_LEN 24 >>>> >>>> Shorten the name of this macro variable, to XLOG_FNAME_LEN, >>>> for more code readability. >> >> Thanks. You have more talent for naming than I do. >> >>>> Comments? >> >> Just reading it again, I think that XLogFileNameById should use >> MAXFNAMELEN, and that XLogFileName should call directly >> XLogFileNameById as both are doing the same operation like in the >> attached. > > We can refactor the code that way, but which looks a bit messy > at least to me. The original coding looks simpler and easier-readable, > so I'd like to adopt the original one here. > >> It seems also safer to use MAXFNAMELEN instead of MAXPGPATH >> for exclusiveCleanupFileName in pg_standby.c and pg_archivecleanup.c. > > Yep.
Pushed. Thanks! Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
