On Tue, 2011-05-03 at 11:30 +0200, Jan Zelený wrote:
Ok, this is corrected set of patches. All your comments were taken
into
account. Also please note that jzeleny-015-2-minor-fixes-in-sysdb.patch which
was acked before has been updated.
> On Wed, 2011-04-20 at 14:24 +0200, Jan Zeleny wrote:
> > Here is a complete set of patches which are needed for the review.
>
> jzeleny-013-2-sysdb-changes-for-sss_cache.patch:
> Nack.
>
> I'd prefer if you used
>
> list->num_dbs = 1;
> list->dbs = talloc_array(list, struct sysdb_ctx, list->num_dbs);
>
> so it's clear from the code that you're creating an array of one
> element.
>
> Also, please split this into two patches. The sysdb_list_init() and
> sysdb_search_netgroups() routines are not logically related and should
> be separate patches.
>
>
>
> jzeleny-014-3-cache-cleaning-utility.patch:
> Nack.
>
> For readability, please use spaces around conditional operators such as
> less-than '<'
>
> After sysdb_search_users(), if it failed, we should 'continue;'. We
> should not fall into the msg_count loop which is uninitialized.
>
> If we can't find SYSDB_NAME in the msg, that should also throw
> ERROR("Couldn't invalidate user"); //No name available
>
> Using sysdb_store_[user|group]() is complete overkill. We don't want to
> be updating the user. This will actually set the expiration to "now + 0"
> which means that you've just introduced a race condition for one second.
> You will want to use sysdb_set_[user|group]_attr() and force the
> expiration time to the value '1'.
>
> Expiration time 0 means "never expire", so your netgroup expiration is
> not going to work at all. Set it to '1'.
>
> If the sysdb_transaction_commit() fails, you MUST attempt to call
> sysdb_transaction_cancel() as well.
>
>
>
>
> jzeleny-015-2-minor-fixes-in-sysdb.patch:
> Ack
>
>
> jzeleny-017-man-page-for-cache-cleanup.patch
> Ack
Ack. Good work.