[SSSD] [PATCH] LDAP: unlink ccname_file_dummy if there is an error

Jakub Hrozek jhrozek at redhat.com
Wed Mar 4 18:54:43 UTC 2015


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 



More information about the sssd-devel mailing list