On Fri, Oct 21, 2022 at 02:10:37PM +0200, Drouvot, Bertrand wrote: > On 10/21/22 2:58 AM, Michael Paquier wrote: >> The same approach with keywords first, regex, and exact match could be >> applied as well for the databases? Perhaps it is just mainly a matter >> of taste, > > Yeah, I think it is.
;) Still it looks that this makes for less confusion with a minimal footprint once the new additions are in place. >> In the same fashion as load_ident(), it seems to me that we >> need two extra things for this patch: >> - if !ok (see where we do MemoryContextDelete(hbacxt)), we need to go >> through new_parsed_lines and free for each line the AuthTokens for the >> database and user lists. >> - if ok and new_parsed_lines != NIL, the same cleanup needs to >> happen. > > Right, but I think that should be "parsed_hba_lines != NIL". For the second case, where we need to free the past contents after a success, yes. > Right. To avoid code duplication in the !ok/ok cases, the function > free_hba_line() has been added in v2: it goes through the list of databases > and roles tokens and call free_auth_token() for each of them. Having a small-ish routine for that is fine. I have spent a couple of hours doing a pass over v2, playing manually with regex patterns, reloads, the system views and item lists. The logic was fine, but I have adjusted a few things related to the comments and the documentation (particularly with the examples, removing one example and updating one with a regex that has a comma, needing double quotes). The CI and all my machines were green, and the test coverage looked sufficient. So, applied. I'll keep an eye on the buildfarm. -- Michael
signature.asc
Description: PGP signature
