2020-02-07 15:22:26

by Steve Dickson

[permalink] [raw]
Subject: [PATCH] query_krb5_ccache: Removed dead code that was flagged by a covscan

Signed-off-by: Steve Dickson <[email protected]>
---
utils/gssd/krb5_util.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index bff759f..a1c43d2 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -1066,8 +1066,6 @@ query_krb5_ccache(const char* cred_cache, char **ret_princname,
*ret_realm = strdup(str+1);
}
k5_free_unparsed_name(context, princstring);
- } else {
- found = 0;
}
}
krb5_free_principal(context, principal);
--
2.24.1


2020-02-07 16:10:49

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] query_krb5_ccache: Removed dead code that was flagged by a covscan



On 2/7/20 10:21 AM, Steve Dickson wrote:
> Signed-off-by: Steve Dickson <[email protected]>
Committed... (tag: nfs-utils-2-4-3-rc7)

steved.
> ---
> utils/gssd/krb5_util.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
> index bff759f..a1c43d2 100644
> --- a/utils/gssd/krb5_util.c
> +++ b/utils/gssd/krb5_util.c
> @@ -1066,8 +1066,6 @@ query_krb5_ccache(const char* cred_cache, char **ret_princname,
> *ret_realm = strdup(str+1);
> }
> k5_free_unparsed_name(context, princstring);
> - } else {
> - found = 0;
> }
> }
> krb5_free_principal(context, principal);
>

2020-02-07 17:26:00

by Olga Kornievskaia

[permalink] [raw]
Subject: Re: [PATCH] query_krb5_ccache: Removed dead code that was flagged by a covscan

On Fri, Feb 7, 2020 at 10:22 AM Steve Dickson <[email protected]> wrote:
>
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> utils/gssd/krb5_util.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
> index bff759f..a1c43d2 100644
> --- a/utils/gssd/krb5_util.c
> +++ b/utils/gssd/krb5_util.c
> @@ -1066,8 +1066,6 @@ query_krb5_ccache(const char* cred_cache, char **ret_princname,
> *ret_realm = strdup(str+1);
> }
> k5_free_unparsed_name(context, princstring);
> - } else {
> - found = 0;
> }

Uhm, sorry wasn't fast enough for you commit decision but I don't see
that this a dead code? krb5_unparse_string() could return an error so
"else" is a valid condition. I mean it's probably unlikely that
check_for_tgt() returns found and they you can't parse the principal
name out of it. But things like memory errors could still be valid
error conditions?


> }
> krb5_free_principal(context, principal);
> --
> 2.24.1
>

2020-02-08 09:21:34

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] query_krb5_ccache: Removed dead code that was flagged by a covscan



On 2/7/20 12:25 PM, Olga Kornievskaia wrote:
> On Fri, Feb 7, 2020 at 10:22 AM Steve Dickson <[email protected]> wrote:
>>
>> Signed-off-by: Steve Dickson <[email protected]>
>> ---
>> utils/gssd/krb5_util.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
>> index bff759f..a1c43d2 100644
>> --- a/utils/gssd/krb5_util.c
>> +++ b/utils/gssd/krb5_util.c
>> @@ -1066,8 +1066,6 @@ query_krb5_ccache(const char* cred_cache, char **ret_princname,
>> *ret_realm = strdup(str+1);
>> }
>> k5_free_unparsed_name(context, princstring);
>> - } else {
>> - found = 0;
>> }
>
> Uhm, sorry wasn't fast enough for you commit decision but I don't see
> that this a dead code? krb5_unparse_string() could return an error so
> "else" is a valid condition. I mean it's probably unlikely that
> check_for_tgt() returns found and they you can't parse the principal
> name out of it. But things like memory errors could still be valid
> error conditions?
Sorry for being so quick with the commit...

The covscan complained "warning: Value stored to 'found' is never read"
which was true... after setting found = 0, found was never used.

Yes, the "else" is a valid condition but not necessary since
setting 'found' to zero does not do anything...

steved.

>
>
>> }
>> krb5_free_principal(context, principal);
>> --
>> 2.24.1
>>
>

2020-02-08 16:07:41

by Olga Kornievskaia

[permalink] [raw]
Subject: Re: [PATCH] query_krb5_ccache: Removed dead code that was flagged by a covscan

On Sat, Feb 8, 2020 at 4:21 AM Steve Dickson <[email protected]> wrote:
>
>
>
> On 2/7/20 12:25 PM, Olga Kornievskaia wrote:
> > On Fri, Feb 7, 2020 at 10:22 AM Steve Dickson <[email protected]> wrote:
> >>
> >> Signed-off-by: Steve Dickson <[email protected]>
> >> ---
> >> utils/gssd/krb5_util.c | 2 --
> >> 1 file changed, 2 deletions(-)
> >>
> >> diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
> >> index bff759f..a1c43d2 100644
> >> --- a/utils/gssd/krb5_util.c
> >> +++ b/utils/gssd/krb5_util.c
> >> @@ -1066,8 +1066,6 @@ query_krb5_ccache(const char* cred_cache, char **ret_princname,
> >> *ret_realm = strdup(str+1);
> >> }
> >> k5_free_unparsed_name(context, princstring);
> >> - } else {
> >> - found = 0;
> >> }
> >
> > Uhm, sorry wasn't fast enough for you commit decision but I don't see
> > that this a dead code? krb5_unparse_string() could return an error so
> > "else" is a valid condition. I mean it's probably unlikely that
> > check_for_tgt() returns found and they you can't parse the principal
> > name out of it. But things like memory errors could still be valid
> > error conditions?
> Sorry for being so quick with the commit...
>
> The covscan complained "warning: Value stored to 'found' is never read"
> which was true... after setting found = 0, found was never used.
>
> Yes, the "else" is a valid condition but not necessary since
> setting 'found' to zero does not do anything...

Got it. Thanks Steve for the explanation.

>
> steved.
>
> >
> >
> >> }
> >> krb5_free_principal(context, principal);
> >> --
> >> 2.24.1
> >>
> >
>