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
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@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
kerr = krb5_cc_destroy(cc->context, cc->ccache); if (kerr) {-- 1.8.3.1
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
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@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.
On (10/09/13 10:37), 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@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
No problem.
LS
On Tue, Sep 10, 2013 at 12:29:22PM +0200, Lukas Slebodnik wrote:
On (10/09/13 10:37), 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@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
No problem.
LS
ACK
On Tue, Sep 10, 2013 at 03:59:29PM +0200, Jakub Hrozek wrote:
On Tue, Sep 10, 2013 at 12:29:22PM +0200, Lukas Slebodnik wrote:
On (10/09/13 10:37), 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@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
No problem.
LS
ACK
Pushed to master.
sssd-devel@lists.fedorahosted.org