Lars Hanke <l...@lhanke.de> writes:

> this solves the LDAP problem. Since it changes some core code, it should
> be tested in other set-ups.

Hm.

I think that Kerberos library functions must all return krb5_error_code,
which should be 0 if there is no error.  So:

    (kcontext && kcontext->db_context
              && ((krb5_dal_handle *) kcontext->db_context)->db_context)

is non-zero (the value of the last pointer, in fact) if all of those
pointers are initialized.  Taking the negation will turn that into a 0 (or
into a 1 if any of them were zero, making the whole expression zero).

So I believe the original function here:

> -krb5_error_code
> +char
> krb5_db_inited(krb5_context kcontext)
> {
> - return !(kcontext && kcontext->db_context &&
> + return (kcontext && kcontext->db_context &&
> ((kdb5_dal_handle *) kcontext->db_context)->db_context);
> }

would return 0 or non-zero, so the original code here:

> /* Check whether database is inited. Open is commented */
> - if ((kerror = krb5_db_inited(context)))
> - return(kerror);
> + if (! krb5_db_inited(context) )
> + return(KRB5_KDB_DBNOTINITED);

should have worked largely the same.  It aborts with an error if
krb5_db_inited returns anything other than zero, which is the correct
behavior.

But, your change makes it work.  So let's see which change was critical.
With your change, krb5_db_inited returns the db_context pointer converted
to a char if everything is initialized, and 0 (NULL converted to a char)
if not.  Therefore, the function returns true on initialization and false
otherwise, the opposite of the original, and you inverted the logic of the
call site, so that's no semantic change.

I suspect the real difference here, though, is that you don't just return
the results of krb5_db_inited; you instead return an explicit error code.
Originally, if things weren't initialized, krb5_db_inited would return an
error code of "1".  Perhaps that's what's not being handled correctly?

If I'm right, reverting your patch and instead changing krb5_db_inited to
the following code should also fix the problem:

krb5_error_code
krb5_db_inited(krb5_context kcontext)
{
    if (kcontext == NULL || kcontext->db_context == NULL)
        return(KRB5_KDB_DBNOTINITED);
    if (((kdb5_dal_handle *) kcontext->db_context)->db_context == NULL)
        return(KRB5_KDB_DBNOTINITED);
    return(0);
}

(which is also more readable and less weird, IMO).

Could you try that and see if I'm right?

-- 
Russ Allbery (r...@debian.org)               <http://www.eyrie.org/~eagle/>



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to