URL:
https://github.com/SSSD/sssd/pull/235
Title: #235: Allow using the "shortnames" feature without requiring any
configuration from the client side
pbrezina commented:
"""
Looking at the first patch, it is a bad habit to assign to an output variable on failures.
Whenever possible, we are trying to avoid it by duplicating the variable for local use and
assigning to it only in success branch. I.e. not:
```c
errno_t
cache_req_domain_new_list_from_domain_resolution_order([...] struct cache_req_domain
**_cr_domains)
{
errno_t ret;
*_cr_domains = NULL;
[...]
*_cr_domains = cache_req_domain_new_list_from_string_list(mem_ctx, domains, list);
if (*_cr_domains == NULL) {
ret = ENOMEM;
goto done;
}
ret = EOK;
done:
talloc_free(tmp_ctx);
return ret;
}
```
But:
```c
errno_t
cache_req_domain_new_list_from_domain_resolution_order([...] struct cache_req_domain
**_cr_domains)
{
struct cache_req_domain *cr_domains;
errno_t ret;
cr_domains = cache_req_domain_new_list_from_string_list(mem_ctx, domains, list);
if (cr_domains == NULL) {
ret = ENOMEM;
goto done;
}
*_cr_domains = cr_domains;
ret = EOK;
done:
talloc_free(tmp_ctx);
return ret;
}
```
Besides this little nitpick, the patches look good to me.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/235#issuecomment-296156663