On Tue, Oct 25, 2022 at 03:43:21PM +0900, Michael Paquier wrote:
>
> Another advantage is that it minimizes the presence of the hardcoded
> HbaFileName and IdentFileName in hba.c, which is one thing we are
> trying to achieve here for the inclusion of more files. I found a bit
> strange that IdentLine had no sourcefile, actually. We track the file
> number but use it nowhere, and it seems to me that having more
> symmetry between both would be a good thing.
If IdentLine->linenumber is useless, why not get rid of it rather than tracking
another useless info? Hba is much more structured so we need a more
specialized struct, but I don't think ident will ever go that way.
> So, the key of the logic is how we are going to organize the
> tokenization of the HBA and ident lines through all the inclusions..
> As far as I get it, tokenize_auth_file() is the root call and
> tokenize_file_with_context() with its depth is able to work on each
> individual file, and it can optionally recurse depending on what's
> included. Why do you need to switch to the old context in
> tokenize_file_with_context()? Could it be simpler to switch once to
> linecxt outside of the internal routine?
It just seemed the cleanest way to go. We could do without but then we would
have to duplicate MemoryContextSwitchTo calls all over the place, and probably
handling an optional memory context creation in the function.
> It looks like GetDirConfFiles() is another piece that can be
> refactored and reviewed on its own, as we use it in
> ParseConfigDirectory()@guc.c.
I'm fine with it.