GNU bug report logs - #65224
mailcap-viewer-passes-test’s caching interferes with precomputed tests

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 65224 in the body.
You can then email your comments to 65224 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Felix Dietrich <felix.dietrich <at> sperrhaken.name>
To: bug-gnu-emacs <bug-gnu-emacs <at> gnu.org>
Subject: mailcap-viewer-passes-test’s caching interferes
 with precomputed tests
Date: Fri, 11 Aug 2023 12:48:49 +0200

[Message part 1 (text/plain, inline)]

When called with a VIEWER-INFO that does not contain a test,
‘mailcap-viewer-passes-test’ erroneously returns nil if it had
previously been called with a VIEWER-INFO whose test was explicitly set
to nil.  As a consequence ‘mailcap-mime-info’ (used e.g. by
‘gnus-mime-view-part-externally’) may return the wrong viewer.

The reason is that ‘mailcap-viewer-passes-test’ adds tests that are
explicitly nil (i.e. (test . nil)) to the cache as (nil nil) (first the
test, second the result) and checks for the non-existence of a test only
after it has tried to find the test result in the cache.  The test
variable for a VIEWER-INFO without a test is nil, and, therefore, the
assoc lookup for nil done on the cache yields (nil nil) and, hence, nil
(the cadr) as the test result.

Moving the check for the non-existence of a test before the cache lookup
fixes the issue; this is what the attached patch does.  In the log
message the bug number (Bug#) needs to be filled in.

[0001-Make-mailcap-viewer-passes-test-return-t-for-viewers.patch (text/x-diff, attachment)]

[Message part 3 (text/plain, inline)]

The following example calls ‘mailcap-viewer-passes-test’ four times.
For the first, second and third call the VIEWER-INFO does not contain a
test; for the third the test is nil.  The results show that the first
and second call correctly return t; the third with (test . nil)
correctly returns nil and adds (nil nil) to the cache; the fourth, then,
incorrectly returns nil.

     #+HEADER: :results table
     #+begin_src emacs-lisp
       (let (mailcap-viewer-test-cache
             (viewer-infos (list (list (cons 'viewer "viewer-w/o-test"))
                                 (list (cons 'viewer "viewer-w/o-test"))
                                 (list (cons 'viewer "viewer-w/-test-nil")
                                       (cons 'test nil))
                                 (list (cons 'viewer "viewer-w/o-test")))))
         (cl-list* (list "Index" "Viewer" "Passes Test" "Cache after Call")
                   'hline
                   (cl-loop for i = 1 then (1+ i)
                            for vi in viewer-infos
                            collect (list (format "%i." i)
                                          (alist-get 'viewer vi)
                                          (mailcap-viewer-passes-test vi nil)
                                          mailcap-viewer-test-cache))))
     #+end_src

     #+RESULTS:
     | Index | Viewer             | Passes Test | Cache after Call |
     |-------+--------------------+-------------+------------------|
     |    1. | viewer-w/o-test    | t           | nil              |
     |    2. | viewer-w/o-test    | t           | nil              |
     |    3. | viewer-w/-test-nil | nil         | ((nil nil))      |
     |    4. | viewer-w/o-test    | nil         | ((nil nil))      |


-- 
Felix Dietrich

Message #12 received at 65224-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Felix Dietrich <felix.dietrich <at> sperrhaken.name>
Cc: 65224-done <at> debbugs.gnu.org
Subject: Re: bug#65224: mailcap-viewer-passes-test’s caching interferes with precomputed tests
Date: Fri, 8 Sep 2023 10:20:52 -0700

Version: 30.1

Felix Dietrich <felix.dietrich <at> sperrhaken.name> writes:

> When called with a VIEWER-INFO that does not contain a test,
> ‘mailcap-viewer-passes-test’ erroneously returns nil if it had
> previously been called with a VIEWER-INFO whose test was explicitly set
> to nil.  As a consequence ‘mailcap-mime-info’ (used e.g. by
> ‘gnus-mime-view-part-externally’) may return the wrong viewer.
>
> The reason is that ‘mailcap-viewer-passes-test’ adds tests that are
> explicitly nil (i.e. (test . nil)) to the cache as (nil nil) (first the
> test, second the result) and checks for the non-existence of a test only
> after it has tried to find the test result in the cache.  The test
> variable for a VIEWER-INFO without a test is nil, and, therefore, the
> assoc lookup for nil done on the cache yields (nil nil) and, hence, nil
> (the cadr) as the test result.
>
> Moving the check for the non-existence of a test before the cache lookup
> fixes the issue; this is what the attached patch does.  In the log
> message the bug number (Bug#) needs to be filled in.

LGTM, pushed to master as commit 6d8458571f7.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.