Thomas Munro <thomas.mu...@gmail.com> writes: >> On Wed, Sep 2, 2020 at 4:35 PM Tom Lane <t...@sss.pgh.pa.us> wrote: >>> Hm. If we do this, I can see wanting to apply the knowledge in more >>> places than walkdir().
> Good idea. Here's a new version that defines a new function > get_dirent_type() in src/common/file_utils_febe.c and uses it for both > frontend and backend walkdir(). Quick thoughts on this patch: * The API spec for get_dirent_type() needs to say that errno is meaningful when the return value is PGFILETYPE_ERROR. That's something that would not be hard to break, so not documenting the point at all doesn't seem great. More generally, I don't especially like having the callers know that the errno is from stat() rather than something else. * I don't quite like the calling code you have that covers some return values and then has a default: case without any comment. It's not really obvious that the default: case is expected to be hit in non-error situations, especially when there is a separate switch path for errors. I can't find fault with the code as such, but I think it'd be good to have a comment there. Maybe along the lines of "Ignore file types other than regular files and directories". Both of these concerns would abate if we had get_dirent_type() just throw an error itself when stat() fails, thereby removing the PGFILETYPE_ERROR result code. I'm not 100% sold either way on that, but it's something to think about. Is there ever going to be a reason for the caller to ignore an error? regards, tom lane