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. 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
