On Tue, Mar 13, 2018 at 9:48 PM, Michael Paquier <mich...@paquier.xyz> wrote:
> On Tue, Mar 13, 2018 at 01:16:27PM +0300, Anastasia Lubennikova wrote:
>> Since these patches contain mostly cosmetic changes and do not break
>> anything, I think it's fine to mark this thread as Ready For Committer
>> without long discussion.
>
> Thanks Anastasia for the review.  The refactoring is quite intuitive I
> think, still that's perhaps a bit too much intrusive.  Extra opinions
> about that are welcome.

Personally it looks very intrusive, so I'm feeling inclined to push
the changes without that refactoring.

The 0005 patch doesn't seem to be right. The patch changes process_source_file()
so that it excludes some directories like pg_notify from the filemap. However,
after that, process_source_file() is called for the files under those "excluded"
directories and then they are not excluded. For example, pg_notify/0000 file is
unexpectedly included in the filemap and copied by pg_rewind.

This problem happens because recurse_dir() has the following code and
ISTM you forgot to take into account it.

    else if (S_ISDIR(fst.st_mode))
    {
        callback(path, FILE_TYPE_DIRECTORY, 0, NULL);
        /* recurse to handle subdirectories */
        recurse_dir(datadir, path, callback);
    }

Regards,

-- 
Fujii Masao

Reply via email to