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.
signature.asc
Description: This is a digitally signed message part
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
