| Message ID | 20200207152109.20855-1-steved@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | query_krb5_ccache: Removed dead code that was flagged by a covscan | expand |
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); >
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 >
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 >> >
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 > >> > > >
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);
Signed-off-by: Steve Dickson <steved@redhat.com> --- utils/gssd/krb5_util.c | 2 -- 1 file changed, 2 deletions(-)