query_krb5_ccache: Removed dead code that was flagged by a covscan
diff mbox series

Message ID 20200207152109.20855-1-steved@redhat.com
State New
Headers show
Series
  • query_krb5_ccache: Removed dead code that was flagged by a covscan
Related show

Commit Message

Steve Dickson Feb. 7, 2020, 3:21 p.m. UTC
Signed-off-by: Steve Dickson <steved@redhat.com>
---
 utils/gssd/krb5_util.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Steve Dickson Feb. 7, 2020, 4:08 p.m. UTC | #1
On 2/7/20 10:21 AM, Steve Dickson wrote:
> Signed-off-by: Steve Dickson <steved@redhat.com>
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);
>
Olga Kornievskaia Feb. 7, 2020, 5:25 p.m. UTC | #2
On Fri, Feb 7, 2020 at 10:22 AM Steve Dickson <steved@redhat.com> wrote:
>
> Signed-off-by: Steve Dickson <steved@redhat.com>
> ---
>  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
>
Steve Dickson Feb. 8, 2020, 9:21 a.m. UTC | #3
On 2/7/20 12:25 PM, Olga Kornievskaia wrote:
> On Fri, Feb 7, 2020 at 10:22 AM Steve Dickson <steved@redhat.com> wrote:
>>
>> Signed-off-by: Steve Dickson <steved@redhat.com>
>> ---
>>  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
>>
>
Olga Kornievskaia Feb. 8, 2020, 4:05 p.m. UTC | #4
On Sat, Feb 8, 2020 at 4:21 AM Steve Dickson <SteveD@redhat.com> wrote:
>
>
>
> On 2/7/20 12:25 PM, Olga Kornievskaia wrote:
> > On Fri, Feb 7, 2020 at 10:22 AM Steve Dickson <steved@redhat.com> wrote:
> >>
> >> Signed-off-by: Steve Dickson <steved@redhat.com>
> >> ---
> >>  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
> >>
> >
>

Patch
diff mbox series

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);