Lars Hanke <[email protected]> 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 ([email protected]) <http://www.eyrie.org/~eagle/>
--
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]
