On Mon, Jul 11, 2022 at 01:25:33PM -0700, Andres Freund wrote:
> On 2022-04-08 13:18:57 -0700, Nathan Bossart wrote:
>> @@ -1035,32 +1036,9 @@ ParseConfigDirectory(const char *includedir,
>>
>> join_path_components(filename, directory, de->d_name);
>> canonicalize_path(filename);
>> - if (stat(filename, &st) == 0)
>> + de_type = get_dirent_type(filename, de, true, elevel);
>> + if (de_type == PGFILETYPE_ERROR)
>> {
>> - if (!S_ISDIR(st.st_mode))
>> - {
>> - /* Add file to array, increasing its size in
>> blocks of 32 */
>> - if (num_filenames >= size_filenames)
>> - {
>> - size_filenames += 32;
>> - filenames = (char **)
>> repalloc(filenames,
>> -
>> size_filenames * sizeof(char *));
>> - }
>> - filenames[num_filenames] = pstrdup(filename);
>> - num_filenames++;
>> - }
>> - }
>> - else
>> - {
>> - /*
>> - * stat does not care about permissions, so the most
>> likely reason
>> - * a file can't be accessed now is if it was removed
>> between the
>> - * directory listing and now.
>> - */
>> - ereport(elevel,
>> - (errcode_for_file_access(),
>> - errmsg("could not stat file \"%s\":
>> %m",
>> - filename)));
>> record_config_file_error(psprintf("could not stat file
>> \"%s\"",
>>
>> filename),
>>
>> calling_file, calling_lineno,
>> @@ -1068,6 +1046,18 @@ ParseConfigDirectory(const char *includedir,
>> status = false;
>> goto cleanup;
>> }
>> + else if (de_type != PGFILETYPE_DIR)
>> + {
>> + /* Add file to array, increasing its size in blocks of
>> 32 */
>> + if (num_filenames >= size_filenames)
>> + {
>> + size_filenames += 32;
>> + filenames = (char **) repalloc(filenames,
>> +
>> size_filenames * sizeof(char *));
>> + }
>> + filenames[num_filenames] = pstrdup(filename);
>> + num_filenames++;
>> + }
>> }
>>
>> if (num_filenames > 0)
>
> Seems like the diff would be easier to read if it didn't move code around as
> much?
I opted to reorganize in order save an indent and simplify the conditions a
bit. The alternative is something like this:
if (de_type != PGFILETYPE_ERROR)
{
if (de_type != PGTFILETYPE_DIR)
{
...
}
}
else
{
...
}
I don't feel strongly one way or another, and I'll change it if you think
it's worth it to simplify the diff.
> Looks pretty reasonable, I'd be happy to commit it, I think.
Appreciate it.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
