On Mon, 2011-06-27 at 18:31 -0400, Jakub Hrozek wrote:
0002: ACK
0003: NACK
The defattr in "%files -n libipa_hbac-devel" is missing a comma before
the last parameter, it says "%defattr(-,root,root-)".
Whoops, I could have sworn I fixed that...
hbac_free_error(): I wonder if it makes sense to add an explicit
"if
(!error) return". I think it is expected that most free functions accept
NULL as a noop and it is quite useful for goto-cleanup cases.
Yeah, that was an oversight. I converted it from talloc_free() (while
eliminating the talloc requirement) and didn't think about the NULL
case.
0004: ACK
0005: ACK
0006: ACK
0007: ACK,
altough the description of ipa_hbac_refresh was added to
config/SSSDConfig.py in patch 0008. Feel free to fix or not, I don't
think this should be a nack as the patchset will be applied as a whole
anyway.
Oops. Looks like I squashed that into the wrong patch. Fixed.
For the rest of the patches - the code seems to be OK sans the one
little issue below. I will defer the "architectural" review to Simo as
the original comments were based on his PAC development experience.
0008: NACK
Please free tmp_ctx in the "immediate:" label in
ipa_hbac_update_groups_send()
Thanks, I missed that.
The "/* Not found as a user; Check Groups*/" seems to be
misleading, the
code never checks users.
That was a relic from before I decided to live with IPA-isms. Originally
I was doing a sysdb_search_user() to eliminate users from consideration.
I've removed this comment.
0009: ACK
0010: ACK
0011: ACK
0012: ACK
Updated patches attached.