Re: [v9.2] Add GUC sepgsql.client_label

On 2012-02-23 12:17, Kohei KaiGai wrote:
> 2012/2/20 Yeb Havinga<yebhavinga@gmail.com>:
>> On 2012-02-05 10:09, Kohei KaiGai wrote:
>>> The attached part-1 patch moves related routines from hooks.c to label.c
>>> because of references to static variables. The part-2 patch implements above
>>> mechanism.
>>
>> I took a short look at this patch but am stuck getting the regression test
>> to run properly.
>>
>> First, patch 2 misses the file sepgsql.sql.in and therefore the creation
>> function command for sepgsql_setcon is missing.
>>
> Thanks for your comments.
>
> I added the definition of sepgsql_setcon function to sepgsql.sql.in file,
> in addition to patch rebasing.

Very brief comments due to must leave keyboard soon:

I read the source code and played a bit with setcon and the debugger, no 
strange things found.

Code comments / questions:

this comment below is a lie, because setcon is set by 
sepgsql_xact_callback()
maybe client_label_committed is a better name for client_label_setcon?

static char *client_label_setcon    = NULL;    /* set by sepgsql_setcon() */

Is the double negation in the sentence below intended?

+ * Neither of them has no special state, the security label being 
initialized
+ * at database-logon time shall be returned.

Is the assert client_label_peer != NULL in sepgsql_get_client_label 
necessary?

sepgsql_set_client_label(), maybe add a comment to !new_label that it is 
reset to the peer label.

new_label == NULL / pending_label->label == NULL means use the peer 
label. Why not use the peer label instead?

set_label: if new_label == current label according to getcon, is it 
necessary to add to the pending list?

sepgsql_subxact_callback(), could this be made easier to read by just 
taking llast(client_label_pending), assert that plabel->subid == mySubId 
and then list_delete on pointer of that listcell?

Some comments contain typos, I can spend some time on this, though I'm 
not a native english speaker so it won't be perfect.

regards,
Yeb Havinga

-- 
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data