[PATCH] check_cc_validity: make sure _valid is always set

[Sumit Bose]

Hi,

this patch fixes a compiler warning in the latest master.

bye,
Sumit

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Fri 23 Aug 2013 04:19:33 AM EDT, Sumit Bose wrote:
> Hi,
>
> this patch fixes a compiler warning in the latest master.

I made that change intentionally so that it would point out if there
was any place in the code that was using _valid when the function
returned anything other than EOK. If you're getting that warning, it
probably means that some consuming function is misbehaving. Could you
fix the bug there instead?
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.14 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iEYEARECAAYFAlIXWK8ACgkQeiVVYja6o6O8fgCgpITIkuCoH3HaYEepcOx1FBQt
> h/8An1QdUMOYqDtsJmAeQiT2I5kNLTaH
> =WBIq
> -----END PGP SIGNATURE-----

[Sumit Bose]

On Fri, Aug 23, 2013 at 08:42:23AM -0400, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On Fri 23 Aug 2013 04:19:33 AM EDT, Sumit Bose wrote:
> > Hi,
> >
> > this patch fixes a compiler warning in the latest master.
>
> I made that change intentionally so that it would point out if there
> was any place in the code that was using _valid when the function
> returned anything other than EOK. If you're getting that warning, it
> probably means that some consuming function is misbehaving. Could you
> fix the bug there instead?

from check_cc_validity():

krberr = krb5_cc_resolve(context, location, &ccache);
    if (krberr == KRB5_FCC_NOFILE || ccache == NULL) {
        /* KRB5_FCC_NOFILE would be returned if the directory components
         * of the DIR cache do not exist, which is the case in /run
         * after a reboot
         */
        DEBUG(SSSDBG_TRACE_FUNC,
              ("ccache %s is missing or empty\n", location));
        valid = false;
        ret = EOK;
        goto done;
    } else if (krberr != 0) {
....
    ret = EOK;
    *_valid = valid;

done:
     if (ccache) krb5_cc_close(context, ccache);
     krb5_free_context(context);
     return ret;

In the KRB5_FCC_NOFILE case ret = EOK but _valid is not set. I think the
new patch might meet your intension.

bye,
Sumit
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.14 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iEYEARECAAYFAlIXWK8ACgkQeiVVYja6o6O8fgCgpITIkuCoH3HaYEepcOx1FBQt
> h/8An1QdUMOYqDtsJmAeQiT2I5kNLTaH
> =WBIq
> -----END PGP SIGNATURE-----

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/23/2013 09:04 AM, Sumit Bose wrote:
> On Fri, Aug 23, 2013 at 08:42:23AM -0400, Stephen Gallagher wrote: 
> On Fri 23 Aug 2013 04:19:33 AM EDT, Sumit Bose wrote:
> > > > Hi,
> > > >
> > > > this patch fixes a compiler warning in the latest master.
>
> I made that change intentionally so that it would point out if
> there was any place in the code that was using _valid when the
> function returned anything other than EOK. If you're getting that
> warning, it probably means that some consuming function is
> misbehaving. Could you fix the bug there instead?
> > from check_cc_validity():
> > krberr = krb5_cc_resolve(context, location, &ccache); if (krberr
> > == KRB5_FCC_NOFILE || ccache == NULL) { /* KRB5_FCC_NOFILE would
> > be returned if the directory components * of the DIR cache do not
> > exist, which is the case in /run * after a reboot */ 
> > DEBUG(SSSDBG_TRACE_FUNC, ("ccache %s is missing or empty\n",
> > location)); valid = false; ret = EOK; goto done; } else if
> > (krberr != 0) { .... ret = EOK; *_valid = valid;
> > done: if (ccache) krb5_cc_close(context, ccache); 
> > krb5_free_context(context); return ret;
> > In the KRB5_FCC_NOFILE case ret = EOK but _valid is not set. I
> > think the new patch might meet your intension.
> > bye, Sumit

Ack. Thanks, I missed the other success case.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.14 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iEYEARECAAYFAlIXXjIACgkQeiVVYja6o6NazACdHll+4/y5o+g+8Yo+3ghR9jxD
> qGQAoK/tp7BsYzvWFDNdJSuUDLjpaVYk
> =RUT/
> -----END PGP SIGNATURE-----

[Jakub Hrozek]

On Fri, Aug 23, 2013 at 09:05:54AM -0400, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 08/23/2013 09:04 AM, Sumit Bose wrote:
> > On Fri, Aug 23, 2013 at 08:42:23AM -0400, Stephen Gallagher wrote: 
> > On Fri 23 Aug 2013 04:19:33 AM EDT, Sumit Bose wrote:
> > > > > Hi,
> > > > >
> > > > > this patch fixes a compiler warning in the latest master.
> >
> > I made that change intentionally so that it would point out if
> > there was any place in the code that was using _valid when the
> > function returned anything other than EOK. If you're getting that
> > warning, it probably means that some consuming function is
> > misbehaving. Could you fix the bug there instead?
> > > from check_cc_validity():
> > > krberr = krb5_cc_resolve(context, location, &ccache); if (krberr
> > > == KRB5_FCC_NOFILE || ccache == NULL) { /* KRB5_FCC_NOFILE would
> > > be returned if the directory components * of the DIR cache do not
> > > exist, which is the case in /run * after a reboot */ 
> > > DEBUG(SSSDBG_TRACE_FUNC, ("ccache %s is missing or empty\n",
> > > location)); valid = false; ret = EOK; goto done; } else if
> > > (krberr != 0) { .... ret = EOK; *_valid = valid;
> > > done: if (ccache) krb5_cc_close(context, ccache); 
> > > krb5_free_context(context); return ret;
> > > In the KRB5_FCC_NOFILE case ret = EOK but _valid is not set. I
> > > think the new patch might meet your intension.
> > > bye, Sumit
>
> Ack. Thanks, I missed the other success case.

Pushed to master sssd-1-10