-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 08/27/2013 03:22 PM, Simo Sorce wrote:
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.
I think you sent the wrong patch 0002 here, Simo.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird -
http://www.enigmail.net/
iEYEARECAAYFAlIdDHYACgkQeiVVYja6o6O1OQCglB7U8L67PQTP/MuvDd6Ld/W4
0cQAnRrYLo6hww+jSulLuaxZsVKvRQ8N
=nt3N
-----END PGP SIGNATURE-----