[SSSD] [PATCH] krb5: Fix warning sometimes uninitialized

Jakub Hrozek jhrozek at redhat.com
Tue Sep 10 08:56:19 UTC 2013


On Tue, Sep 10, 2013 at 10:37:47AM +0200, Sumit Bose wrote:
> On Tue, Sep 10, 2013 at 10:13:10AM +0200, Lukas Slebodnik wrote:
> > ehlo,
> > 
> > clang found a warning in simo's krb5 refactoring patches.
> > 
> > src/providers/krb5/krb5_utils.c:850:9: warning: variable 'ret' is used uninitialized whenever
> >       'if' condition is false [-Wsometimes-uninitialized]
> >     if (kerr) {
> >         ^~~~
> > src/providers/krb5/krb5_utils.c:859:12: note: uninitialized use occurs here
> >     return ret;
> >            ^~~
> > src/providers/krb5/krb5_utils.c:850:5: note: remove the 'if' if its condition is always true
> >     if (kerr) {
> >     ^~~~~~~~~~
> > src/providers/krb5/krb5_utils.c:847:16: note: initialize the variable 'ret' to silence this
> >       warning
> >     errno_t ret;
> > 
> > LS
> 
> > From fbb64181ffbc38df657fc2224f60b002e4ecb05c Mon Sep 17 00:00:00 2001
> > From: Lukas Slebodnik <lslebodn at redhat.com>
> > Date: Tue, 10 Sep 2013 10:07:51 +0200
> > Subject: [PATCH] krb5: Fix warning sometimes uninitialized
> > 
> > warning: variable 'ret' is used uninitialized whenever
> > 'if' condition is false
> >     if (kerr) {
> >         ^~~~
> > ---
> >  src/providers/krb5/krb5_utils.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/providers/krb5/krb5_utils.c b/src/providers/krb5/krb5_utils.c
> > index bb933d7f722cbef90b73f4a721382165572c69b9..fb9942c53b17c7de246f893fc25de282a0a116e9 100644
> > --- a/src/providers/krb5/krb5_utils.c
> > +++ b/src/providers/krb5/krb5_utils.c
> > @@ -844,7 +844,7 @@ done:
> >  static errno_t sss_destroy_ccache(struct sss_krb5_ccache *cc)
> >  {
> >      krb5_error_code kerr;
> > -    errno_t ret;
> > +    errno_t ret = EOK;
> >  
> 
> I know it's nit-picking but I would prefer to not initialize ret but set
> it to EOK e.g. in an else clause. The reason is that this way the
> complier can still detect any uninitialized use if sss_destroy_ccache()
> is extended/enhanced in future.
> 
> bye,
> Sumit

I agree this is safer. We've had a bug in the past that went something
like:
    
    errno_t ret = EOK;
    char *ptr;

    ptr = malloc(123);
    if (ptr == NULL) {
        goto done;
    }

If we didn't set the ret to EOK initially, compiler would have warned us
about the unitialized variable.



More information about the sssd-devel mailing list