Hi John, Thanks for having a look at this.
On Wed, 8 Apr 2020 at 00:16, John Naylor <john.nay...@2ndquadrant.com> wrote: > Overall looks good to me. Just a couple things I see: > > It seems _hash_log2 is still in the tree, but has no callers? Yeah, I left it in there since it was an external function. Perhaps we could rip it out and write something in the commit message that it should be replaced with the newer functions. Thinking of extension authors here. > - max_size = 8; /* semi-arbitrary small power of 2 */ > - while (max_size < min_size + LIST_HEADER_OVERHEAD) > - max_size *= 2; > + max_size = pg_nextpower2_32(Max(8, min_size + LIST_HEADER_OVERHEAD)); > > Minor nit: We might want to keep the comment that the number is > "semi-arbitrary" here as well. I had dropped that as the 8 part was mentioned in the comment above: "The minimum allocation is 8 ListCell units". I can put it back, I had just thought it was overkill. > - 'pg_waldump', 'scripts'); > + 'pg_validatebackup', 'pg_waldump', 'scripts'); > > This seems like a separate concern? That's required due to the #include "lib/simplehash.h" in pg_validatebackup.c. I have to say, I didn't really take the time to understand all the Perl code there, but without that change, I was getting a link error when testing on Windows, and after I added pg_validatebackup to that array, it worked. David
