On Tue, 2013-08-27 at 14:05 -0400, Stephen Gallagher wrote:
Patch 0002: Nack
As discussed on IRC, please split out the patch that removes the
contrib patch into a separate patch in the set and modify it so that
it reverts to using the built-in defaults (which means updating the
sssd.spec.in as well).
Ah I had all in one patch, splitting ...
In sss_get_system_ccname_template(), you fail to check the result of
the talloc_strdup(). Also, by convention you should have an explicit
ret = EOK in the success case just before the done: label.
It was actually intentional, but your comment made me think that we
probably want to fail if the strdup fails, changed.
Wouldn't it make more sense to return ENOENT than a NULL ccname?
from
sss_get_system_ccname_template()? It's not critical, but we tend to
use that approach in the rest of the code. It would be more clear in
the code what you're doing here too. I had to read it again after
reading the consuming function to understand it properly.
I was a bit doubtful because ENOENT always result ina "File not found
error" which is quite misleading. So I added a new sssd error of
ERR_NOT_FOUND, hpe this is ok.
Simo.
--
Simo Sorce * Red Hat, Inc * New York