Re: [HACKERS] pg_hba_file_settings view patch
От | Michael Paquier |
---|---|
Тема | Re: [HACKERS] pg_hba_file_settings view patch |
Дата | |
Msg-id | CAB7nPqRNNR6iuDe_9RCjBFWjg31X_SEynHSw1wZhvwv-LzZ8Og@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: [HACKERS] pg_hba_file_settings view patch (Michael Paquier <michael.paquier@gmail.com>) |
Ответы |
Re: [HACKERS] pg_hba_file_settings view patch
(Haribabu Kommi <kommi.haribabu@gmail.com>)
|
Список | pgsql-hackers |
On Thu, Jan 5, 2017 at 1:58 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > Could you hold on a bit to commit that? I'd like to look at it in more > details. At quick glance, there is for example no need to use > CreateTemplateTupleDesc and list the columns both in pg_proc.h and the > C routine itself. And memset() can be used in fill_hba_line for the > error code path. And here we go. +<programlisting> +postgres=# select * from pg_hba_rules; [... large example ...] + +</programlisting> It would be nice to reduce the width of this example. That's not going to be friendly with the generated html. + switch (hba->conntype) + { + case ctLocal: + values[index] = CStringGetTextDatum("local"); + break; + case ctHost: + values[index] = CStringGetTextDatum("host"); + break; + case ctHostSSL: + values[index] = CStringGetTextDatum("hostssl"); + break; + case ctHostNoSSL: + values[index] = CStringGetTextDatum("hostnossl"); + break; + default: + elog(ERROR, "unexpected connection type in parsed HBA entry"); + break; + } Here let's remove the break clause and let compilers catch problem when they show up. + if (hba->pamservice) + { + initStringInfo(&str); + appendStringInfoString(&str, "pamservice="); + appendStringInfoString(&str, hba->pamservice); + options[noptions++] = CStringGetTextDatum(str.data); + } There is a bunch of code duplication here. I think that it would make more sense to encapsulate that into a routine, at least let's use appendStringInfo and let's group the two calls together. +/* LDAP supports 10 currently, keep this well above the most any method needs */ +#define MAX_OPTIONS 12 Er, why? There is an assert already, that should be enough. =# \d pg_hba_rules View "pg_catalog.pg_hba_rules" Column | Type | Collation | Nullable | Default ------------------+---------+-----------+----------+---------line_number | integer | | |type | text | | |keyword_database | text[] | | |database | text[] | | |keyword_user | text[] | | |user_name | text[] | | |keyword_address | text | | |address | inet | | |netmask | inet | | |hostname | text | | |method | text | | |options | text[] | | |error | text | | | keyword_database and database map actually to the same thing if you refer to a raw pg_hba.conf file because they have the same meaning for user. You could simplify the view and just remove keyword_database, keyword_user and keyword_address. This would simplify your patch as well with all hte mumbo-jumbo to see if a string is a dedicated keyword or not. In its most simple shape pg_hba_rules should show to the user as an exact map of the entries of the raw file. I have copied the example file of pg_hba.conf, reloaded it: https://www.postgresql.org/docs/devel/static/auth-pg-hba-conf.html And then the output result gets corrupted by showing up free()'d results: null | null | \x7F\x7F\x7F\x7F\x7F + if (err_msg) + { + /* type */ + index++; + nulls[index] = true; [... long sequence ...] Please let's use MemSet here and remove this large chunk of code... + if (!superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + (errmsg("must be superuser to view pg_hba.conf settings")))); Access to the function is already revoked, so what's the point of this superuser check? + tupdesc = CreateTemplateTupleDesc(NUM_PG_HBA_LOOKUP_ATTS, false); + TupleDescInitEntry(tupdesc, (AttrNumber) 1, "line_number", + INT4OID, -1, 0); There is no need to list all the columns here. You can just use get_call_result_type() and be done with it as the types and columns names are already listed in the pg_proc entry of the new function. -- Michael
В списке pgsql-hackers по дате отправления: