[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#36834: 27.0.50; [PATCH] password-cache.el: confuses key absence with
From: |
Óscar Fuentes |
Subject: |
bug#36834: 27.0.50; [PATCH] password-cache.el: confuses key absence with nil password |
Date: |
Mon, 29 Jul 2019 16:12:45 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) |
"Basil L. Contovounesios" <contovob@tcd.ie> writes:
>> The change uses gethash instead of intern-soft, but those functions act
>> differently when the password (the value associated with the key) was
>> nil.
>
> Is it valid for the password to be nil? The logic in password-read
> suggests otherwise.
Callers are sending nil. If it is not valid, there is a problem
elsewhere, but my understanding is that a nil password means "no
password" and it is cached in the memoization sense.
>> The effect is that every call to password-cache-add with nil as
>> password creates a new timer,
>
> Where is password-cache-add being passed a nil password?
The caller is auth-source-remember, IIRC, which itself is called from
auth-source-search.
>> and password-in-cache-p returns nil if
>> there exists a (key nil) entry on password-data, when previously it
>> would return non-nil.
>
> I think a nil key is also not expected.
(key nil) means a hash table entry with `key' as key and nil as value,
not that key is nil.
> Note that password-in-cache-p is currently identical to
> password-read-from-cache. One can probably be written in terms of the
> other.
Yes, right now they are identical, which causes a problem, because
checking for key existence shall not be the same as retrieving the value
when value can be nil.
> Even if these "memhash" checks are TRT, I suggest either reusing or
> copying the hash table method of map-contains-key, rather than comparing
> against an interned symbol.
Is map-contains-key available by default? I'm wary of introducing new
dependencies for saving just a few characters.