* bug#37095: [PATCH] Save match data in ucs-normalize-region
@ 2019-08-20 7:18 Akinori MUSHA
2019-08-23 3:51 ` Lars Ingebrigtsen
0 siblings, 1 reply; 4+ messages in thread
From: Akinori MUSHA @ 2019-08-20 7:18 UTC (permalink / raw)
To: 37095
[-- Attachment #1.1.1: Type: text/plain, Size: 1581 bytes --]
A patch generated by git format-patch is attached below, which simply
wraps `ucs-normalize-region` with `save-match-data`.
I'm a user of the Emacs Mac port by mituharu was investigating a bug
where dired fails to open a certain local directory on macOS. The
error was raised at `replace-match` in the `insert-directory`
function:
```
(when (re-search-forward "^ *\\(total\\)" nil t)
(let ((available (get-free-disk-space ".")))
(when available
;; Replace "total" with "used", to avoid confusion.
(replace-match "total used in directory" nil nil nil 1)
```
And it turned out the match data changed after returning from
`get-free-disk-space` and that was why `replace-match` failed.
Inside of `get-free-disk-space` most platforms uses a generic method
to get the free space, and that part is fine because it is surrounded
by `save-match-data`. However, the Mac port is one of the few
platforms that implements a native 'file-system-info` function, which
is called if it exists. Then, the `file-system-info` in `src/mac.c`
calls ENCODE_FILE() on a given directory name, which in the end calls
`ucs-normalize-region` to normalize the filename, where the match data
is clobbered.
https://bitbucket.org/mituharu/emacs-mac/src/df827786d7a7fb0a0e2f27577af67e32d9a888a9/src/mac.c#lines-2337
ENCODE_FILE() is transparently called by many C functions, which means
`ucs-normalize-region` can be called at unpredictable timings, so I
think it should keep match data unchanged.
--
Akinori MUSHA / https://akinori.org/
[-- Attachment #1.1.2: 0001-Save-match-data-in-ucs-normalize-region.patch --]
[-- Type: text/plain, Size: 2416 bytes --]
From 11d49adcbadcfcbe844f873e63ba1d596f72e4c5 Mon Sep 17 00:00:00 2001
From: Akinori MUSHA <knu@idaemons.org>
Date: Mon, 19 Aug 2019 23:53:50 +0900
Subject: [PATCH] Save match data in ucs-normalize-region
* lisp/international/ucs-normalize.el (ucs-normalize-region): Save
match data. This function can be transparently called via
ENCODE_FILE() when the underlying filesytem uses normalized Unicode
filenames, so it requires much care not to cause any side effects.
---
lisp/international/ucs-normalize.el | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/lisp/international/ucs-normalize.el b/lisp/international/ucs-normalize.el
index 6f1e770c09..d02d4b8e3e 100644
--- a/lisp/international/ucs-normalize.el
+++ b/lisp/international/ucs-normalize.el
@@ -514,19 +514,20 @@ ucs-normalize-region
(narrow-to-region from to)
(goto-char (point-min))
(let (start-pos starter)
- (while (re-search-forward quick-check-regexp nil t)
- (setq starter (string-to-char (match-string 0)))
- (setq start-pos (match-beginning 0))
- (ucs-normalize-block
- ;; from
- (if (or (= start-pos (point-min))
- (and (= 0 (ucs-normalize-ccc starter))
- (not (memq starter ucs-normalize-combining-chars))))
- start-pos (1- start-pos))
- ;; to
- (if (looking-at ucs-normalize-combining-chars-regexp)
- (match-end 0) (1+ start-pos))
- translation-table composition-predicate))))))
+ (save-match-data
+ (while (re-search-forward quick-check-regexp nil t)
+ (setq starter (string-to-char (match-string 0)))
+ (setq start-pos (match-beginning 0))
+ (ucs-normalize-block
+ ;; from
+ (if (or (= start-pos (point-min))
+ (and (= 0 (ucs-normalize-ccc starter))
+ (not (memq starter ucs-normalize-combining-chars))))
+ start-pos (1- start-pos))
+ ;; to
+ (if (looking-at ucs-normalize-combining-chars-regexp)
+ (match-end 0) (1+ start-pos))
+ translation-table composition-predicate)))))))
;; --------------------------------------------------------------------------------
--
2.22.0
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply related [flat|nested] 4+ messages in thread
* bug#37095: [PATCH] Save match data in ucs-normalize-region
2019-08-20 7:18 bug#37095: [PATCH] Save match data in ucs-normalize-region Akinori MUSHA
@ 2019-08-23 3:51 ` Lars Ingebrigtsen
2019-08-23 5:06 ` YAMAMOTO Mitsuharu
0 siblings, 1 reply; 4+ messages in thread
From: Lars Ingebrigtsen @ 2019-08-23 3:51 UTC (permalink / raw)
To: Akinori MUSHA; +Cc: 37095
Akinori MUSHA <knu@iDaemons.org> writes:
> A patch generated by git format-patch is attached below, which simply
> wraps `ucs-normalize-region` with `save-match-data`.
>
> I'm a user of the Emacs Mac port by mituharu was investigating a bug
> where dired fails to open a certain local directory on macOS. The
> error was raised at `replace-match` in the `insert-directory`
> function:
>
> ```
> (when (re-search-forward "^ *\\(total\\)" nil t)
> (let ((available (get-free-disk-space ".")))
> (when available
> ;; Replace "total" with "used", to avoid confusion.
> (replace-match "total used in directory" nil nil nil 1)
> ```
>
> And it turned out the match data changed after returning from
> `get-free-disk-space` and that was why `replace-match` failed.
You don't say what Emacs version you're reporting this bug for, but the
following patch was applied in February 2018 to the Emacs trunk, so I
think this problem has been fixed by now:
diff --git a/lisp/files.el b/lisp/files.el
index 91aa95d631..75d3b7b1e7 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -6473,9 +6473,10 @@ get-free-disk-space
space (normally, the number of free 1KB blocks).
If DIR's free space cannot be obtained, this function returns nil."
- (let ((avail (nth 2 (file-system-info dir))))
- (if avail
- (format "%.0f" (/ avail 1024)))))
+ (save-match-data
+ (let ((avail (nth 2 (file-system-info dir))))
+ (if avail
+ (format "%.0f" (/ avail 1024))))))
;; The following expression replaces `dired-move-to-filename-regexp'.
(defvar directory-listing-before-filename-regexp
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply related [flat|nested] 4+ messages in thread
* bug#37095: [PATCH] Save match data in ucs-normalize-region
2019-08-23 3:51 ` Lars Ingebrigtsen
@ 2019-08-23 5:06 ` YAMAMOTO Mitsuharu
2019-08-23 7:21 ` Akinori MUSHA
0 siblings, 1 reply; 4+ messages in thread
From: YAMAMOTO Mitsuharu @ 2019-08-23 5:06 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: Akinori MUSHA, 37095
On Fri, 23 Aug 2019 12:51:29 +0900,
Lars Ingebrigtsen wrote:
>
> Akinori MUSHA <knu@iDaemons.org> writes:
>
> > A patch generated by git format-patch is attached below, which simply
> > wraps `ucs-normalize-region` with `save-match-data`.
> >
> > I'm a user of the Emacs Mac port by mituharu was investigating a bug
> > where dired fails to open a certain local directory on macOS. The
> > error was raised at `replace-match` in the `insert-directory`
> > function:
> >
> > ```
> > (when (re-search-forward "^ *\\(total\\)" nil t)
> > (let ((available (get-free-disk-space ".")))
> > (when available
> > ;; Replace "total" with "used", to avoid confusion.
> > (replace-match "total used in directory" nil nil nil 1)
> > ```
> >
> > And it turned out the match data changed after returning from
> > `get-free-disk-space` and that was why `replace-match` failed.
>
> You don't say what Emacs version you're reporting this bug for, but the
> following patch was applied in February 2018 to the Emacs trunk, so I
> think this problem has been fixed by now:
For the Mac port, the "work" branch already contains a similar change:
https://bitbucket.org/mituharu/emacs-mac/commits/b651c3a6bab6795202e2ebcd4396d665909cc210
It will shortly be included in the next release based on Emacs 26.3 RC1.
YAMAMOTO Mitsuharu
mituharu@math.s.chiba-u.ac.jp
^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#37095: [PATCH] Save match data in ucs-normalize-region
2019-08-23 5:06 ` YAMAMOTO Mitsuharu
@ 2019-08-23 7:21 ` Akinori MUSHA
0 siblings, 0 replies; 4+ messages in thread
From: Akinori MUSHA @ 2019-08-23 7:21 UTC (permalink / raw)
To: YAMAMOTO Mitsuharu; +Cc: Lars Ingebrigtsen, 37095
[-- Attachment #1: Type: text/plain, Size: 2123 bytes --]
Thanks you both for the information.
I don't insist if you don't think this way, but I think this bug was partly
due to ucs-normalize-region being called only if the user is on a certain
platform/filesystem and the file name actually needs normalization, so it
would be better to fix such a function not to break a globally shared state
like match data so that there's no room for more bugs of the same kind.
Just my two yen.
2019-08-23日(金) 14:06 YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>:
> On Fri, 23 Aug 2019 12:51:29 +0900,
> Lars Ingebrigtsen wrote:
> >
> > Akinori MUSHA <knu@iDaemons.org> writes:
> >
> > > A patch generated by git format-patch is attached below, which simply
> > > wraps `ucs-normalize-region` with `save-match-data`.
> > >
> > > I'm a user of the Emacs Mac port by mituharu was investigating a bug
> > > where dired fails to open a certain local directory on macOS. The
> > > error was raised at `replace-match` in the `insert-directory`
> > > function:
> > >
> > > ```
> > > (when (re-search-forward "^ *\\(total\\)" nil t)
> > > (let ((available (get-free-disk-space ".")))
> > > (when available
> > > ;; Replace "total" with "used", to avoid confusion.
> > > (replace-match "total used in directory" nil nil nil
> 1)
> > > ```
> > >
> > > And it turned out the match data changed after returning from
> > > `get-free-disk-space` and that was why `replace-match` failed.
> >
> > You don't say what Emacs version you're reporting this bug for, but the
> > following patch was applied in February 2018 to the Emacs trunk, so I
> > think this problem has been fixed by now:
>
> For the Mac port, the "work" branch already contains a similar change:
>
>
> https://bitbucket.org/mituharu/emacs-mac/commits/b651c3a6bab6795202e2ebcd4396d665909cc210
>
> It will shortly be included in the next release based on Emacs 26.3 RC1.
>
> YAMAMOTO Mitsuharu
> mituharu@math.s.chiba-u.ac.jp
>
>
[-- Attachment #2: Type: text/html, Size: 3054 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-08-23 7:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-20 7:18 bug#37095: [PATCH] Save match data in ucs-normalize-region Akinori MUSHA
2019-08-23 3:51 ` Lars Ingebrigtsen
2019-08-23 5:06 ` YAMAMOTO Mitsuharu
2019-08-23 7:21 ` Akinori MUSHA
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.