bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[Gregory Heytings]

[-- Attachment #1: Type: text/plain, Size: 5308 bytes --]


(Disclaimer: I have no knowledge whatsoever about the ISO-2022-JP 
encoding, and although this looks like a bug, I'm not sure this is 
actually a bug; I report this at the suggesion of Eli in bug#46859.)

I downloaded the file [1], and converted it to the ISO-2022-JP encoding 
with iconv -t iso-2022-jp one.txt > iso-2022-jp.txt.  The resulting file 
is attached to this bug report.  It ends with two CRLFs, at byte offsets 
2993 and 2995.  However, after emacs -Q iso-2022-jp.txt, with M-: 
(goto-char (filepos-to-bufferpos POS 'exact)) we get:

POS = 2991, 2992: last but one visible character (HIRAGANA LETTER RU)
POS = 2993, 2994: last visible character (IDEOGRAPHIC FULL STOP)
POS = 2995, 2996: first CRLF
POS = 2997: second CRLF
POS = 2998: point-max
POS = 2999: first CRLF
POS = 3000, 3001: second CRLF
POS >= 3002: point-max

I would have expected:

POS = 2989, 2990: last but one visible character (HIRAGANA LETTER RU)
POS = 2991, 2992: last visible character (IDEOGRAPHIC FULL STOP)
POS = 2993, 2994: first CRLF
POS = 2995, 2996: second CRLF
POS >= 2997: point-max

The opposite operation M-: (bufferpos-to-filepos (- (point) POS) 'exact) 
apparently also has bugs; its return values are not coherent with the 
above ones:

POS = 0: 3003
POS = 1: 3001
POS = 2: 2999
POS = 3 (IDEOGRAPHIC FULL STOP): 2997
POS = 4 (HIRAGANA LETTER RU): 2995

I would have expected:

POS = 0: 2997
POS = 1: 2995
POS = 2: 2993
POS = 3 (IDEOGRAPHIC FULL STOP): 2991
POS = 4 (HIRAGANA LETTER RU): 2989

[1] https://darza.com/ecbackend/vendor/symfony/mime/Tests/Fixtures/samples/charsets/iso-2022-jp/one.txt

In GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.23, cairo version 1.16.0)
  of 2020-11-08, modified by Debian built on x86-ubc-01
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: Debian GNU/Linux bullseye/sid

Configured using:
  'configure --build x86_64-linux-gnu --prefix=/usr
  --sharedstatedir=/var/lib --libexecdir=/usr/lib
  --localstatedir=/var/lib --infodir=/usr/share/info
  --mandir=/usr/share/man --enable-libsystemd --with-pop=yes
  --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/27.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/27.1/site-lisp:/usr/share/emacs/site-lisp
  --with-sound=alsa --without-gconf --with-mailutils --build
  x86_64-linux-gnu --prefix=/usr --sharedstatedir=/var/lib
  --libexecdir=/usr/lib --localstatedir=/var/lib
  --infodir=/usr/share/info --mandir=/usr/share/man --enable-libsystemd
  --with-pop=yes
  --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/27.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/27.1/site-lisp:/usr/share/emacs/site-lisp
  --with-sound=alsa --without-gconf --with-mailutils --with-cairo
  --with-x=yes --with-x-toolkit=gtk3 --with-toolkit-scroll-bars
  'CFLAGS=-g -O2
  -fdebug-prefix-map=/build/emacs-6jKC2B/emacs-27.1+1=. -fstack-protector-strong
  -Wformat -Werror=format-security -Wall' 'CPPFLAGS=-Wdate-time
  -D_FORTIFY_SOURCE=2' LDFLAGS=-Wl,-z,relro'

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF
ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS LIBSYSTEMD
JSON PDUMPER LCMS2 GMP

Important settings:
   value of $LANG: en_US.UTF-8
   locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
   tooltip-mode: t
   global-eldoc-mode: t
   eldoc-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   line-number-mode: t
   transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame minibuffer cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
cairo move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

[-- Attachment #2: Type: text/plain, Size: 2997 bytes --]

ISO-2022-JP^[$B$O!"%$%s%?!<%M%C%H>e^[(B(^[$BFC$KEE;R%a!<%k^[(B)^[$B$J$I$G;H$o$l$kF|K\$NJ8;zMQ$NJ8;zId9f2=J}<0!#^[(BISO/IEC 2022^[$B$N%(%9%1!<%W%7!<%1%s%9$rMxMQ$7$FJ8;z=89g$r@Z$jBX$($k^[(B7^[$B%S%C%H$N%3!<%I$G$"$k$3$H$rFCD'$H$9$k^[(B (^[$B%"%J%&%s%95!G=$N%(%9%1!<%W%7!<%1%s%9$O>JN,$5$l$k^[(B)^[$B!#B/$K!V^[(BJIS^[$B%3!<%I!W$H8F$P$l$k$3$H$b$"$k!#^[(B

^[$B35MW^[(B
^[$BF|K\8lI=5-$X$NMxMQ$,A[Dj$5$l$F$$$kJ8;z%3!<%I$G$"$j!"F|K\8l$NMxMQ$5$l$k%M%C%H%o!<%/$K$*$$$F!"F|K\$N5,3J$r1~MQ$7$?$b$N$G$"$k!#$^$?J8;z=89g$H$7$F$O!"F|K\8l$GMQ$$$i$l$k4A;z!"$R$i$,$J!"%+%?%+%J$O$b$A$m$s!"%i%F%sJ8;z!"%.%j%7%"J8;z!"%-%j%kJ8;z$J$I$b4^$s$G$*$j!"3X=Q$d;:6H$NJ,Ln$G$NMxMQ$b9MN8$?$b$N$H$J$C$F$$$k!#5,3JL>$K!"^[(BISO^[$B$NF|K\8l$N8@8l%3!<%I$G$"$k^[(Bja^[$B$G$O$J$/!"9q!&CO0hL>%3!<%I$N^[(BJP^[$B$,<($5$l$F$$$k$f$($s$G$"$k!#^[(B
^[$BJ8;z=89g$H$7$F^[(BJIS X 0201^[$B$N^[(BC0^[$B=89g!J@)8fJ8;z!K!"^[(BJIS X 0201^[$B$N%i%F%sJ8;z=89g!"^[(BISO 646^[$B$N9q:]4p=`HG?^7AJ8;z!"^[(BJIS X 0208^[$B$N^[(B1978^[$BG/HG!J^[(BJIS C 6226-1978^[$B!K$H^[(B1983^[$BG/$*$h$S^[(B1990^[$BG/HG$,MxMQ$G$-$k!#^[(BJIS X 0201^[$B$NJR2>L>J8;z=89g$OMxMQ$G$-$J$$!#^[(B1986^[$BG/0J9_!"F|K\$NEE;R%a!<%k$GMQ$$$i$l$F$-$?^[(BJUNET^[$B%3!<%I$r!"B<0f=c!&^[(BMark Crispin^[$B!&^[(BErik van der Poel^[$B$,^[(B1993^[$BG/$K^[(BRFC^[$B2=$7$?$b$N^[(B(RFC 1468)^[$B!#8e$K^[(BJIS X 0208:1997^[$B$NImB0=q^[(B2^[$B$H$7$F^[(BJIS^[$B$K5,Dj$5$l$?!#^[(BMIME^[$B$K$*$1$kJ8;zId9f2=J}<0$N<1JLMQ$NL>A0$H$7$F^[(B IANA ^[$B$KEPO?$5$l$F$$$k!#^[(B
^[$B$J$*!"Id9f2=$N;EMM$K$D$$$F$O^[(BISO/IEC 2022#ISO-2022-JP^[$B$b;2>H!#^[(B

ISO-2022-JP^[$B$HHsI8=`E*3HD%;HMQ^[(B
^[$B!V^[(BJIS^[$B%3!<%I!W!J$^$?$O!V^[(BISO-2022-JP^[$B!W!K$H$$$&%3!<%IL>$N5,Dj2<$G$O!"$=$N;EMMDL$j$N;HMQ$,5a$a$i$l$k!#$7$+$7!"^[(BWindows OS^[$B>e$G$O!"<B:]$K$O^[(BCP932^[$B%3!<%I^[(B (Microsoft^[$B$K$h$k^[(BShift JIS^[$B$r3HD%$7$?0!<o!#^[(BISO-2022-JP^[$B5,Dj30J8;z$,DI2C$5$l$F$$$k!#!K$K$h$kFH<+3HD%!J$NJ8;z!K$rCG$j$J$/;H$&%"%W%j%1!<%7%g%s$,B?$$!#$3$NNc$H$7$F^[(BInternet Explorer^[$B$d^[(BOutlook Express^[$B$,$"$k!#$^$?!"^[(BEmEditor^[$B!"=(4]%(%G%#%?$d^[(BThunderbird^[$B$N$h$&$J^[(BMicrosoft^[$B<R0J30$N^[(BWindows^[$B%"%W%j%1!<%7%g%s$G$bF1MM$N>l9g$,$"$k!#$3$N>l9g!"^[(BISO-2022-JP^[$B$NHO0O30$NJ8;z$r;H$C$F$7$^$&$H!"0[$J$k@=IJ4V$G$OL$Dj5AITL@J8;z$H$7$FG'<1$5$l$k$+!"$b$7$/$OJ8;z2=$1$r5/$3$9860x$H$J$k!#$=$N$?$a!"^[(BWindows^[$BMQ$NEE;R%a!<%k%/%i%$%"%s%H$G$"$C$F$bFH<+3HD%$NJ8;z$r;HMQ$9$k$H7Y9p$r=P$7$?$j!"$"$($F;H$($J$$$h$&$K@)8B$7$F$$$k$b$N$bB8:_$9$k!#$5$i$K$O^[(BISO-2022-JP^[$B$NHO0OFb$G$"$C$F$b^[(BCP932^[$B$OHsI8=`J8;z!J^[(BFULLWIDTH TILDE^[$BEy!K$r;}$D$N$GJ8;z2=$1$N860x$K$J$jF@$k!#^[(B
^[$B$^$?!"Id9f2=J}<0L>$r^[(BISO-2022-JP^[$B$H$7$F$$$k$N$K!"J8;z=89g$H$7$F$O^[(BJIS X 0212 (^[$B$$$o$f$kJd=u4A;z^[(B) ^[$B$d^[(BJIS X 0201^[$B$NJR2>L>J8;z=89g^[(B (^[$B$$$o$f$kH>3Q%+%J^[(B) ^[$B$r$bId9f2=$7$F$$$kNc$,$"$k$,!"^[(BISO-2022-JP^[$B$G$O$3$l$i$NJ8;z$r5vMF$7$F$$$J$$!#$3$l$i$NId9f2=$OFH<+3HD%$N<BAu$G$"$j!"Cf$K$O^[(BISO/IEC 2022^[$B$N;EMM$K=`5r$9$i$7$F$$$J$$$b$N$b$"$k^[(B[2]^[$B!#=>$C$F<u?.B&$NEE;R%a!<%k%/%i%$%"%s%H$,$3$l$i$NFH<+3HD%$KBP1~$7$F$$$J$$>l9g!"$=$NJ8;z$"$k$$$O$=$NJ8;z$r4^$`9T!";~$K$O%F%-%9%HA4BN$,J8;z2=$1$9$k$3$H$,$"$k!#^[(B

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[Eli Zaretskii]

> Date: Thu, 04 Mar 2021 21:21:24 +0000
> From: Gregory Heytings <gregory@heytings.org>
> 
> (Disclaimer: I have no knowledge whatsoever about the ISO-2022-JP 
> encoding, and although this looks like a bug, I'm not sure this is 
> actually a bug; I report this at the suggesion of Eli in bug#46859.)
> 
> I downloaded the file [1], and converted it to the ISO-2022-JP encoding 
> with iconv -t iso-2022-jp one.txt > iso-2022-jp.txt.  The resulting file 
> is attached to this bug report.  It ends with two CRLFs, at byte offsets 
> 2993 and 2995.  However, after emacs -Q iso-2022-jp.txt, with M-: 
> (goto-char (filepos-to-bufferpos POS 'exact)) we get:
> 
> POS = 2991, 2992: last but one visible character (HIRAGANA LETTER RU)
> POS = 2993, 2994: last visible character (IDEOGRAPHIC FULL STOP)
> POS = 2995, 2996: first CRLF
> POS = 2997: second CRLF
> POS = 2998: point-max
> POS = 2999: first CRLF
> POS = 3000, 3001: second CRLF
> POS >= 3002: point-max
> 
> I would have expected:
> 
> POS = 2989, 2990: last but one visible character (HIRAGANA LETTER RU)
> POS = 2991, 2992: last visible character (IDEOGRAPHIC FULL STOP)
> POS = 2993, 2994: first CRLF
> POS = 2995, 2996: second CRLF
> POS >= 2997: point-max
> 
> The opposite operation M-: (bufferpos-to-filepos (- (point) POS) 'exact) 
> apparently also has bugs; its return values are not coherent with the 
> above ones:
> 
> POS = 0: 3003
> POS = 1: 3001
> POS = 2: 2999
> POS = 3 (IDEOGRAPHIC FULL STOP): 2997
> POS = 4 (HIRAGANA LETTER RU): 2995
> 
> I would have expected:
> 
> POS = 0: 2997
> POS = 1: 2995
> POS = 2: 2993
> POS = 3 (IDEOGRAPHIC FULL STOP): 2991
> POS = 4 (HIRAGANA LETTER RU): 2989
> 
> [1] https://darza.com/ecbackend/vendor/symfony/mime/Tests/Fixtures/samples/charsets/iso-2022-jp/one.txt

There's something strange going on here with encoding of the buffer
using iso-2022-jp-dos: near the end of the encoded bytestream, between
the encoded HIRAGANA LETTER KO (こ) and HIRAGANA LETTER TO (と), we
get 6 extra bytes: "ESC ( B ESC $ B".  AFAIU, this sequence mean
switch to ASCII and then switch back to Japanese.  So together these 6
bytes are a no-op as regards to their effect on the text, but they
disrupt the logic of filepos-to-bufferpos because they introduce extra
bytes that aren't there in the original file.

Kenichi, why are these 6 bytes inserted by encode-coding-region, but
not when we encode the same text as part of saving the buffer to its
file?  And why does it happen near the end of the text, between those
2 particular letters?

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[handa]

In article <83ft0obk7i.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:

> Kenichi, why are these 6 bytes inserted by encode-coding-region, but
> not when we encode the same text as part of saving the buffer to its
> file?  And why does it happen near the end of the text, between those
> 2 particular letters?

There surely exists a bug.  Could you please try the attached patch?

The reason why that bug did not happen on file writing is that the code
in write_region calls encoding routine repeatedly without
CODING_MODE_LAST_BLOCK flag, and only in the case that flushing is
required (e.g. the case of iso-2022-jp), just for flushing, it calls
enoding routine again with CODING_MODE_LAST_BLOCK flag.  In that case,
carryover does not happen in encode_coding ().

---
K. Handa
handa@gnu.org

diff --git a/src/coding.c b/src/coding.c
index 221a9cad89..a9d5a7ccdc 100644
--- a/src/coding.c
+++ b/src/coding.c
@@ -7799,7 +7799,14 @@ encode_coding (struct coding_system *coding)
     coding_set_source (coding);
     consume_chars (coding, translation_table, max_lookup);
     coding_set_destination (coding);
+    /* If consume_chars did not consume all source chars, we call
+       coding->encoder again in the next iteration, and thus, for this
+       iteration, we must clear CODING_MODE_LAST_BLOCK flag.  */
+    unsigned saved_mode = coding->mode;
+    if (coding->consumed_char < coding->src_chars)
+      coding->mode &= ~CODING_MODE_LAST_BLOCK;
     (*(coding->encoder)) (coding);
+    coding->mode = saved_mode;
   } while (coding->consumed_char < coding->src_chars);
 
   if (BUFFERP (coding->dst_object) && coding->produced_char > 0)

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[Eli Zaretskii]

> From: handa <handa@gnu.org>
> Cc: gregory@heytings.org, 46933@debbugs.gnu.org
> Date: Sat, 27 Mar 2021 14:38:56 +0900
> 
> In article <83ft0obk7i.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Kenichi, why are these 6 bytes inserted by encode-coding-region, but
> > not when we encode the same text as part of saving the buffer to its
> > file?  And why does it happen near the end of the text, between those
> > 2 particular letters?
> 
> There surely exists a bug.  Could you please try the attached patch?
> 
> The reason why that bug did not happen on file writing is that the code
> in write_region calls encoding routine repeatedly without
> CODING_MODE_LAST_BLOCK flag, and only in the case that flushing is
> required (e.g. the case of iso-2022-jp), just for flushing, it calls
> enoding routine again with CODING_MODE_LAST_BLOCK flag.  In that case,
> carryover does not happen in encode_coding ().

Thanks.  The patch fixes the problem with the extra 6 bytes, so I
installed it.

The results of filepos-to-bufferpos with the file attached by Gregory
are better now, but there are still problems for some values of BYTE
argument.  The problem is that ISO-2022 encoding (and others like it)
include shift-in and shift-out sequences, used to switch between
character sets.  As a trivial example, each CR+LF sequence has the
"ESC ( B" sequence before it and "ESC $ B" sequence after it, to
switch to ASCII before the newline, then switch to Japanese after it.
And likewise whenever there's Latin text within Japanese (there are
quite a lot of them in this particular file).  These shift-in and
shift-out sequences consume bytes, but don't produce any characters.
So if the BYTE argument of filepos-to-bufferpos specifies a byte in
the middle of one of these shift sequences, the result will be
incorrect, because decoding a partial sequence produces the bytes of
that sequence verbatim, and the logic in filepos-to-bufferpos of using
the length of the decoded text breaks.  We need special handling of
this and other similar coding-systems to fix these corner use cases,
similarly to what we do in filepos-to-bufferpos--dos.  Patches
welcome.

I'm leaving this bug open because not all of the problem was fixed.

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[handa]

In article <8335whowuj.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:
> Thanks.  The patch fixes the problem with the extra 6 bytes, so I
> installed it.

Thank you for the improved concise comment in the code.

> The results of filepos-to-bufferpos with the file attached by Gregory
> are better now, but there are still problems for some values of BYTE
> argument.  The problem is that ISO-2022 encoding (and others like it)
> include shift-in and shift-out sequences, used to switch between
> character sets.  As a trivial example, each CR+LF sequence has the
> "ESC ( B" sequence before it and "ESC $ B" sequence after it, to
> switch to ASCII before the newline, then switch to Japanese after it.
> And likewise whenever there's Latin text within Japanese (there are
> quite a lot of them in this particular file).  These shift-in and
> shift-out sequences consume bytes, but don't produce any characters.
> So if the BYTE argument of filepos-to-bufferpos specifies a byte in
> the middle of one of these shift sequences, the result will be
> incorrect, because decoding a partial sequence produces the bytes of
> that sequence verbatim, and the logic in filepos-to-bufferpos of using
> the length of the decoded text breaks.  We need special handling of
> this and other similar coding-systems to fix these corner use cases,
> similarly to what we do in filepos-to-bufferpos--dos.  Patches
> welcome.

How about something like this method:
1. Encode the buffer text one line by one until we get a longer byte
sequence than BYTE.
2. Delete the result of enoding the last line above.
3. Provided that the above last line has chars C1 C2 ... Cn, 
encode characters C1...Cn, C1...Cn-1, C1...Cn-2 until we get a shorter
byte sequence than BYTE.

The first step may be optimized by encode multiple lines instead of
single line.

---
K. Handa
handa@gnu.org

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[Eli Zaretskii]

> From: handa <handa@gnu.org>
> Cc: gregory@heytings.org, 46933@debbugs.gnu.org
> Date: Sat, 27 Mar 2021 22:23:59 +0900
> 
> How about something like this method:
> 1. Encode the buffer text one line by one until we get a longer byte
> sequence than BYTE.
> 2. Delete the result of enoding the last line above.
> 3. Provided that the above last line has chars C1 C2 ... Cn, 
> encode characters C1...Cn, C1...Cn-1, C1...Cn-2 until we get a shorter
> byte sequence than BYTE.
> 
> The first step may be optimized by encode multiple lines instead of
> single line.

Even if we do optimize, this would be very slow, I think.  And what if
the buffer has no newlines?

In any case, the problem is not with encoding, the problem is with
decoding.  Encoding doesn't have this problem because we always encode
more than enough (we use the value of BYTE as the count of
_characters_ to encode, so for ISO-2022 encoding it is usually much
more than needed).  By contrast, when decoding, we decode exactly
BYTE+1 bytes, which then hits the problem if that offset is inside a
shift sequence.

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[handa]

In article <83pmzkog6x.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:

> > How about something like this method:
> > 1. Encode the buffer text one line by one until we get a longer byte
> > sequence than BYTE.
> > 2. Delete the result of enoding the last line above.
> > 3. Provided that the above last line has chars C1 C2 ... Cn, 
> > encode characters C1...Cn, C1...Cn-1, C1...Cn-2 until we get a shorter
> > byte sequence than BYTE.
> > 
> > The first step may be optimized by encode multiple lines instead of
> > single line.

> Even if we do optimize, this would be very slow, I think.

Whether it is too slow or not depends on what filepos-to-bufferpos is
used for.  Do you know why filepos-to-bufferpos (and
bufferpos-to-filepos) is introduced?

> And what if the buffer has no newlines?

In that case, just do the step 2.  Or, we can use the bi-sectioning
technique.

> In any case, the problem is not with encoding, the problem is with
> decoding.  Encoding doesn't have this problem because we always encode
> more than enough (we use the value of BYTE as the count of
> _characters_ to encode, so for ISO-2022 encoding it is usually much
> more than needed).  By contrast, when decoding, we decode exactly
> BYTE+1 bytes, which then hits the problem if that offset is inside a
> shift sequence.

Then, that implementation should be changed.

Any coding system can have :post-read-conversion and
:pre-write-conversion functions, it is not guaranteed that encoded byte
length is greater than the number of characters.

---
K. Handa
handa@gnu.org

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[Eli Zaretskii]

> From: handa <handa@gnu.org>
> Cc: gregory@heytings.org, 46933@debbugs.gnu.org
> Date: Sun, 28 Mar 2021 23:29:41 +0900
> 
> > In any case, the problem is not with encoding, the problem is with
> > decoding.  Encoding doesn't have this problem because we always encode
> > more than enough (we use the value of BYTE as the count of
> > _characters_ to encode, so for ISO-2022 encoding it is usually much
> > more than needed).  By contrast, when decoding, we decode exactly
> > BYTE+1 bytes, which then hits the problem if that offset is inside a
> > shift sequence.
> 
> Then, that implementation should be changed.
> 
> Any coding system can have :post-read-conversion and
> :pre-write-conversion functions, it is not guaranteed that encoded byte
> length is greater than the number of characters.

Agreed, but AFAICT, ISO-2022-JP doesn't have any of these attributes,
right?

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[handa]

In article <83tuovmivc.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:

> > Any coding system can have :post-read-conversion and
> > :pre-write-conversion functions, it is not guaranteed that encoded byte
> > length is greater than the number of characters.

> Agreed, but AFAICT, ISO-2022-JP doesn't have any of these attributes,
> right?

Yes, but one can add them by coding-system-put.

By the way, what is the intention of filepos-to-bufferpos?  Why that
function was introduce?

---
K. Handa
handa@gnu.org

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[Eli Zaretskii]

> From: handa <handa@gnu.org>
> Cc: gregory@heytings.org, 46933@debbugs.gnu.org
> Date: Fri, 02 Apr 2021 00:14:02 +0900
> 
> By the way, what is the intention of filepos-to-bufferpos?  Why that
> function was introduce?

The original (and so far the only) use case was an Info manual
separated into several files, where the tag table at the end of the
main file specifies offsets in bytes.  See the function
Info-find-node-2 in info.el.

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[Eli Zaretskii]

> From: handa <handa@gnu.org>
> Cc: gregory@heytings.org, 46933@debbugs.gnu.org
> Date: Fri, 02 Apr 2021 00:14:02 +0900
> 
> In article <83tuovmivc.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:
> 
> > > Any coding system can have :post-read-conversion and
> > > :pre-write-conversion functions, it is not guaranteed that encoded byte
> > > length is greater than the number of characters.
> 
> > Agreed, but AFAICT, ISO-2022-JP doesn't have any of these attributes,
> > right?
> 
> Yes, but one can add them by coding-system-put.

Leaving the :pre-write/:post-read-conversion use case aside, do we
have some means of find where ISO-2022 shift-in/out sequence begins
and ends, so that we never try to decode a partial sequence (and
produce "characters" that are not really in the original buffer)?
If not, where can I find the description of every kind of such
sequences, i.e. sequences that modify the decoder state without
producing any characters?

(UTF-8 has the same issue, btw, but in that case we have a simpler
solution.)

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[handa]

In article <83zgyif2aq.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:

> Leaving the :pre-write/:post-read-conversion use case aside, do we
> have some means of find where ISO-2022 shift-in/out sequence begins
> and ends, so that we never try to decode a partial sequence (and
> produce "characters" that are not really in the original buffer)?
> If not, where can I find the description of every kind of such
> sequences, i.e. sequences that modify the decoder state without
> producing any characters?

The official definition is in the standard ISO/IEC 2022, but
it seems that this wiki page:
  https://en.wikipedia.org/wiki/ISO/IEC_2022
is more concise.  Emacs implements all control sequences shown in the
sections: "Shift functions", "Character set designations", and
"Interaction with other coding systems".

> > By the way, what is the intention of filepos-to-bufferpos?  Why that
> > function was introduce?

> The original (and so far the only) use case was an Info manual
> separated into several files, where the tag table at the end of the
> main file specifies offsets in bytes.  See the function
> Info-find-node-2 in info.el.

As filepos-to-bufferpos accepts the optional arg CODING-SYSTEM,
I've thought BYTE arg is:
  a byte position in a file that will be created by encoding the current
  buffer by CODING-SYSTEM

But it seems that the usage in Info-find-node-2 is:
  a byte position in an existing file that may not be created by Emacs

There's a case that they are different.  The method I wrote in the
previous mail works only in the former case.   And it seems that the
current implementation of filepos-to-bufferpos is the same because it
tries to get byte sequence by encode-coding-region.

For the latter case, perhaps something like the following code works.

;; Return the buffer position correspoinding to the byte position
;; FILEPOS in FILE provided that FILE is decoded by CODING-SYSTEM.
(defun temp (file filepos coding-system)
  (with-temp-buffer
    (set-buffer-multibyte nil)
    (insert-file-contents-literally file)
    (let ((full (decode-coding-region 1 (point-max) coding-system t))
	  partial)
      (while (and (setq partial (decode-coding-region 1 (1+ filepos)
						      coding-system t))
		  (not (eq (compare-strings full 0 (length partial)
					    partial 0 (length partial))
			   t)))
	  (setq filepos (1+ filepos)))
      (1+ (length partial)))))

If it is too slow, there are a few ways to make it faster.

---
K. Handa
handa@gnu.org

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[Lars Ingebrigtsen]

handa <handa@gnu.org> writes:

> But it seems that the usage in Info-find-node-2 is:
>   a byte position in an existing file that may not be created by Emacs
>
> There's a case that they are different.  The method I wrote in the
> previous mail works only in the former case.   And it seems that the
> current implementation of filepos-to-bufferpos is the same because it
> tries to get byte sequence by encode-coding-region.
>
> For the latter case, perhaps something like the following code works.
>
> ;; Return the buffer position correspoinding to the byte position
> ;; FILEPOS in FILE provided that FILE is decoded by CODING-SYSTEM.
> (defun temp (file filepos coding-system)
>   (with-temp-buffer
>     (set-buffer-multibyte nil)
>     (insert-file-contents-literally file)
>     (let ((full (decode-coding-region 1 (point-max) coding-system t))
> 	  partial)
>       (while (and (setq partial (decode-coding-region 1 (1+ filepos)
> 						      coding-system t))
> 		  (not (eq (compare-strings full 0 (length partial)
> 					    partial 0 (length partial))
> 			   t)))
> 	  (setq filepos (1+ filepos)))
>       (1+ (length partial)))))
>
> If it is too slow, there are a few ways to make it faster.

(I'm going through old bug reports that unfortunately weren't resolved
at the time.)

If I understand correctly, the suggestion here is to use this function
in Info-find-node-2 instead of `filepos-to-bufferpos'?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  gregory@heytings.org,  46933@debbugs.gnu.org
> Date: Mon, 20 Jun 2022 02:59:52 +0200
> 
> handa <handa@gnu.org> writes:
> 
> > For the latter case, perhaps something like the following code works.
> >
> > ;; Return the buffer position correspoinding to the byte position
> > ;; FILEPOS in FILE provided that FILE is decoded by CODING-SYSTEM.
> > (defun temp (file filepos coding-system)
> >   (with-temp-buffer
> >     (set-buffer-multibyte nil)
> >     (insert-file-contents-literally file)
> >     (let ((full (decode-coding-region 1 (point-max) coding-system t))
> > 	  partial)
> >       (while (and (setq partial (decode-coding-region 1 (1+ filepos)
> > 						      coding-system t))
> > 		  (not (eq (compare-strings full 0 (length partial)
> > 					    partial 0 (length partial))
> > 			   t)))
> > 	  (setq filepos (1+ filepos)))
> >       (1+ (length partial)))))
> >
> > If it is too slow, there are a few ways to make it faster.
> 
> (I'm going through old bug reports that unfortunately weren't resolved
> at the time.)
> 
> If I understand correctly, the suggestion here is to use this function
> in Info-find-node-2 instead of `filepos-to-bufferpos'?

"Unless it's too slow".

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> If I understand correctly, the suggestion here is to use this function
>> in Info-find-node-2 instead of `filepos-to-bufferpos'?
>
> "Unless it's too slow".

Let's see...

> The original (and so far the only) use case was an Info manual
> separated into several files, where the tag table at the end of the
> main file specifies offsets in bytes.  See the function
> Info-find-node-2 in info.el.

We no longer split up .info files into several files, so that's a bit
difficult to test.  But there's one new in-tree usage for this -- in
hexl.el.  (In hexl-mode-exit and hexl-maybe-dehexlify-buffer.)  I don't
know whether that has the problem described in this bug report, though
(I'm not familiar with hexl.el at all).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: handa@gnu.org,  gregory@heytings.org,  46933@debbugs.gnu.org
> Date: Tue, 21 Jun 2022 12:40:15 +0200
> 
> > The original (and so far the only) use case was an Info manual
> > separated into several files, where the tag table at the end of the
> > main file specifies offsets in bytes.  See the function
> > Info-find-node-2 in info.el.
> 
> We no longer split up .info files into several files, so that's a bit
> difficult to test.

The GDB manual, if you have it, is generated in split form.

> But there's one new in-tree usage for this -- in
> hexl.el.  (In hexl-mode-exit and hexl-maybe-dehexlify-buffer.)  I don't
> know whether that has the problem described in this bug report, though
> (I'm not familiar with hexl.el at all).

There's no reason why this won't be relevant to hexl: it is a
general-purpose hex editor, so editing a file encoded in one of those
problematic ISO-2022 encodings should bump into the same issues.

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> The GDB manual, if you have it, is generated in split form.

Thanks; I could test with that.

>> But there's one new in-tree usage for this -- in
>> hexl.el.  (In hexl-mode-exit and hexl-maybe-dehexlify-buffer.)  I don't
>> know whether that has the problem described in this bug report, though
>> (I'm not familiar with hexl.el at all).
>
> There's no reason why this won't be relevant to hexl: it is a
> general-purpose hex editor, so editing a file encoded in one of those
> problematic ISO-2022 encodings should bump into the same issues.

I meant more that I don't really know what it wants to achieve, or what
kinds of files are typically used by hexl users.  Do people use that on
huge hex dumps or something else?  And on hex dumps, there's typically
be no coding system, so loading the file into memory just to determine
that would be inefficient.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: handa@gnu.org,  gregory@heytings.org,  46933@debbugs.gnu.org
> Date: Wed, 22 Jun 2022 06:17:09 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> But there's one new in-tree usage for this -- in
> >> hexl.el.  (In hexl-mode-exit and hexl-maybe-dehexlify-buffer.)  I don't
> >> know whether that has the problem described in this bug report, though
> >> (I'm not familiar with hexl.el at all).
> >
> > There's no reason why this won't be relevant to hexl: it is a
> > general-purpose hex editor, so editing a file encoded in one of those
> > problematic ISO-2022 encodings should bump into the same issues.
> 
> I meant more that I don't really know what it wants to achieve, or what
> kinds of files are typically used by hexl users.

Let me explain.  These functions are used in hexl for the case where
you have a file visited "normally" (which decodes it using some
coding-system, as Emacs normally would), then want to run hexl on it
for some reason, and later perhaps go back to editing it "normally".
In this case, hexl tries to keep the buffer position in the same place
across conversion to hexl and back, because that's what the user would
expect.  The coding-system used for that is the one Emacs used to
decode the file's contents when originally visiting the file
"normally".

> Do people use that on huge hex dumps or something else?

Hexl is also useful to look at the exact contents of a file that
displays strangely when visited normally, and that's when these
functions are useful.

Of course, accuracy is less important in this case than in the case of
multi-file Info, so maybe we don't care too much for the hexl case.

bug#46933: Possible bugs in filepos-to-bufferpos / bufferpos-to-filepos

[Gregory Heytings]

>> Kenichi, why are these 6 bytes inserted by encode-coding-region, but 
>> not when we encode the same text as part of saving the buffer to its 
>> file?  And why does it happen near the end of the text, between those 2 
>> particular letters?
>
> There surely exists a bug.  Could you please try the attached patch?
>
> The reason why that bug did not happen on file writing is that the code 
> in write_region calls encoding routine repeatedly without 
> CODING_MODE_LAST_BLOCK flag, and only in the case that flushing is 
> required (e.g. the case of iso-2022-jp), just for flushing, it calls 
> enoding routine again with CODING_MODE_LAST_BLOCK flag.  In that case, 
> carryover does not happen in encode_coding ().
>

Thank you.  I tried the patch, and it seems to fix the 
bufferpos-to-filepos bug, but not the filepos-to-bufferpos one.  On the 
example file,

(bufferpos-to-filepos (- (point-max) POS) 'exact)

gives the expected results:

POS = 0: 2997
POS = 1: 2995
POS = 2: 2993
POS = 3 (IDEOGRAPHIC FULL STOP): 2991
POS = 4 (HIRAGANA LETTER RU): 2989

But (goto-char (filepos-to-bufferpos POS 'exact)) gives:

POS = 2985, 2986: last but one visible character (HIRAGANA LETTER RU)
POS = 2987, 2988: last visible character (IDEOGRAPHIC FULL STOP)
POS = 2989, 2990: first CRLF
POS = 2991: second CRLF
POS = 2992: point-max
POS = 2993: first CRLF
POS = 2994, 2995: second CRLF
POS >= 2996: point-max

where I would have expected:

POS = 2989, 2990: last but one visible character (HIRAGANA LETTER RU)
POS = 2991, 2992: last visible character (IDEOGRAPHIC FULL STOP)
POS = 2993, 2994: first CRLF
POS = 2995, 2996: second CRLF
POS >= 2997: point-max