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

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Felix Dietrich <felix.dietrich <at> sperrhaken.name>
Subject: bug#65224: closed (Re: bug#65224: mailcap-viewer-passes-test’s
 caching interferes with precomputed tests)
Date: Fri, 08 Sep 2023 17:22:02 +0000

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

Your bug report

#65224: mailcap-viewer-passes-test’s caching interferes with precomputed tests

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 65224 <at> debbugs.gnu.org.

-- 
65224: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=65224
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

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.

[Message part 3 (message/rfc822, inline)]

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 4 (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 6 (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

Previous Next

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