Hello Michal,
thank you for quick review. Please see attached patches (only 2nd
changed).
On Wed, 2014-06-18 at 15:58 +0200, Michal Židek wrote:
[snip]
Patch 02:
You should return all errors from the create_dummy_netgr
properly. Here:
- netgr = talloc_zero(step_ctx->nctx, struct getent_ctx);
- if (netgr == NULL) {
- DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zero failed, ignored.\n");
and here
- ret = set_netgroup_entry(step_ctx->nctx, netgr);
- if (ret != EOK) {
- DEBUG(SSSDBG_CRIT_FAILURE, "...");
- }
The caller can than ignore the errors (as it already
does). This is not a big deal, but I think it will be
nicer that way. Currently you only report
talloc_strdup failure to the caller and ignore the other
possible errors, which is a little inconsistent.
I think you are right, in previous patch I was just being careful not to
change anything - to be 1:1 with original code, but I agree that these
changes are for better.
Also , I am not sure if create_dummy_netgr is good name
for the helper function. It creates negative cache entry,
so maybe something like create_negative_netgr?
Renamed, hope you like the new name better.
Thanks,
PR