From: "Óscar Fuentes" <ofv@wanadoo.es>
To: "Basil L. Contovounesios" <contovob@tcd.ie>
Cc: Stefan Monnier <monnier@iro.umontreal.ca>, 36834@debbugs.gnu.org
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 [thread overview]
Message-ID: <87wog1rkte.fsf@telefonica.net> (raw)
In-Reply-To: <87v9vlck5a.fsf@tcd.ie> (Basil L. Contovounesios's message of "Mon, 29 Jul 2019 09:36:17 +0100")
"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.
next prev parent reply other threads:[~2019-07-29 14:12 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-29 5:12 bug#36834: 27.0.50; [PATCH] password-cache.el: confuses key absence with nil password Óscar Fuentes
2019-07-29 8:36 ` Basil L. Contovounesios
2019-07-29 14:12 ` Óscar Fuentes [this message]
2019-08-10 8:19 ` Eli Zaretskii
2019-08-11 16:00 ` Basil L. Contovounesios
2019-08-10 9:14 ` Stefan Monnier
2019-08-10 10:01 ` Eli Zaretskii
2019-08-11 23:45 ` Óscar Fuentes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87wog1rkte.fsf@telefonica.net \
--to=ofv@wanadoo.es \
--cc=36834@debbugs.gnu.org \
--cc=contovob@tcd.ie \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).