On 10/14/2018 06:41 PM, Michael Paquier wrote:
On Sun, Oct 14, 2018 at 10:24:55AM -0400, Andrew Dunstan wrote:
[snip]
Thanks a lot for the review, Andrew!
This code now seems redundant:
if (strcmp(fn, ".") == 0 ||
strcmp(fn, "..") == 0)
return true;
Indeed. I have renamed skipfile() to isRelFileName on the way, which
looks better in the context.
I would probably reverse the order of these two tests. It might not make any
difference, assuming fn is never an empty string, but it seems more logical
to me.
+ /* good to go if only digits */
+ if (fn[pos] == '\0')
+ return false;
+ /* leave if no digits */
+ if (pos == 0)
+ return true;
No objections to that. Done.
It also looks to me like the check for a segment number doesn't ensure
there is at least one digit, so "123." might pass, but I could be
wrong. In any case, there isn't a test for that, and there probably
should be.
You are right here. This name pattern has been failing. Fixed in the
attached. I have added "123_" and "123_." as extra patterns to check.
What do you think about the updated version attached?
Looks good to me.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
