On 04/28/2016 01:39 PM, Pavel Březina wrote:
On 04/22/2016 03:31 PM, Petr Cech wrote:
> Hi,
>
> attached patch set resolves ticket [1].
>
> It is a refactor of negative cache. I prepared patch set step by step,
> so reviewer can do review simpler. If you prefer two bigger patches
> please write me.
>
> Notice: This patch set is applicable after [2]
>
>
> [1]
https://fedorahosted.org/sssd/ticket/2317
> [2]
>
https://www.mail-archive.com/sssd-devel@lists.fedorahosted.org/msg26506.html
>
Hi, just few nitpicks:
Hi Pavel,
thank you for review.
Patch 1 NEGCACHE: Adding timeout to struct sss_nc_ctx
> +int responder_get_neg_timeout_from_confdb(struct confdb_ctx *cdb,
> + uint32_t *neg_timeout)
> +{
> + int value;
> + int ret = EOK;
Please, use errno_t instead of int as a type of return code. And also do
not initialize it with declaration (especially not to EOK), it tends to
hide issues where you forget to set ret (I know it is not this case, but
it is a bad habit).
You're right. I will fix it.
> neg_timeout
I think ncache_timeout would be a better name to use throughout the code
since we shortcut "negative cache" to ncache all over the code and you
remove most of neg_timeout names in following patches.
It could be better name, but this function disappears during next commits.
You can look at
https://github.com/celestian/sssd/commits/negcache_refactor
There are all commits. I sent they in several mails accordingly to their
ticket. And the last three are not on the list because haven't the test
done yet.
I think it make sense to nitpick final shape. How I wrote I can merge
particular patches to two or three bigger.
Regards
--
Petr^4 Čech