Re: pg_hba_file_settings view patch

On Thu, Jan 19, 2017 at 4:08 PM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:

> On Wed, Jan 18, 2017 at 4:11 PM, Haribabu Kommi
> <kommi(dot)haribabu(at)gmail(dot)com> wrote:
> > updated patch attached.
>
> Thanks for the new version.
>
> > Added tap tests patch also attached.
>
> This begins to look really nice. I am having fun torturing it :)
>

Thanks for the review.

Here are I think my last comments:
>
> + linecxt = tokenize_file(HbaFileName, file, &hba_lines,
> &hba_line_nums, &hba_raw_lines);
> + FreeFile(file);
> tokenize_file can leave on ERROR, in which case the file descriptor
> would leak. You much likely need a
> PG_END_ENSURE_ERROR_CLEANUP/PG_ENSURE_ERROR_CLEANUP block here with a
> callback to FreeFile() if an error is caught.
>

Added the cleanup mechanism. But the tokenize_file() function call
present in many places, But in one flow still it is possible to have
file descriptor leak because of pg_hba_rules view. Because of this
reason, added the cleanup everywhere.

> + <entry>
> + ADDRESS specifies the set of hosts the record matches.
> + It can be a host name, or it is made up of an IP address
> + or keywords such as (<literal>all</literal>,
> + <literal>samehost</literal> and <literal>samenet</literal>).
> + </entry>
> Why is that upper-case?
>

Corrected.

+ <entry>
> + If not null, an error message indicating why this
> + rule could not be loaded
> + </entry>
> Need a dot here, that's a sentence.
>

updated.

src/test/regress/expected/rules.out needs to be refreshed, regression
> tests are failing.
>

Corrected.

Updated patch attached.

Regards,
Hari Babu
Fujitsu Australia