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 <lsleb...@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.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
