On Wed, Mar 04, 2015 at 02:25:54PM +0100, Jakub Hrozek wrote:
On Fri, Feb 27, 2015 at 12:04:59PM -0700, Daniel Hjorth wrote:
On Fri, Feb 27, 2015 at 10:13:28AM +0100, Jakub Hrozek wrote:
On Fri, Feb 27, 2015 at 09:59:57AM +0100, Sumit Bose wrote:
done:
- talloc_free(tmp_ctx); if (krberr != 0) KRB5_SYSLOG(krberr); if (keytab) krb5_kt_close(context, keytab); if (context) krb5_free_context(context);
- if (ccname_file_dummy) unlink(ccname_file_dummy);
It would be nice if you can check the return code of unlink(). I know it is a bit useless because we can't do anything if unlink() fails. But we try to keep the SSSD build free of warning even with high warning levels and when using static analyzers like Coverity or the one from clang. I would expect that at least some of them will complain about the unchecked return value. Additionally a debug message telling why unlink() failed might help debug issue when the dummy file is not removed properly.
bye, Sumit
That's a good point, I think using the 'ret' variable would be fine, since we return krberr.
The modified patch is attached. I used the same DEBUG that is being used in the rename but changed the failure to MINOR because it doesn't seem like a CRIT to me. Thoughts?
Daniel
ACK
* master: 2b20ff2e33ad3993a9cad910c4b4b828513613df * sssd-1-12: 0b5036e4c652e6983a3352c045c8701d6573587b