Subject: bug#20783: closed (Re: bug#20783: 25.0.50; [PATCH] byte-to-position has internal off-by-one bug)

From: [email-masked] (GNU bug Tracking System)
Date: Wed, 17 Jun 2015 12:30:07 +0000

Your bug report

#20783: 25.0.50; [PATCH] byte-to-position has internal off-by-one bug

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 [email-masked].

-- 
20783: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20783
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#20783: 25.0.50; [PATCH] byte-to-position has internal off-by-one bug
From: Wolfgang Jenkner <[email-masked]>
To: Eli Zaretskii <[email-masked]>
Cc: [email-masked]
Date: Wed, 17 Jun 2015 14:19:10 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

Version: 25.1

On Thu, Jun 11 2015, Eli Zaretskii wrote:

> Works for me, thanks.  But please add a comment there about
> BYTE_TO_CHAR expecting byte positions that are on a character
> boundary, so that the reason for the loop is clear.

Done and pushed.

However, I didn't follow my own suggestion of adding a remark to the
comment above BYTE_TO_CHAR, after all, as it is true for most macros or
functions with a byte position argument, so adding such a comment to
just one of them could be confusing, I think.

Thank you for steering this change in the right direction.

Subject: 25.0.50; [PATCH] byte-to-position has internal off-by-one bug
From: Wolfgang Jenkner <[email-masked]>
To: [email-masked]
Date: Wed, 10 Jun 2015 17:13:30 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

Here's a test case for the bug:

(with-temp-buffer
  (insert "éé")
  (let ((i 1) pos res)
    (while (setq pos (byte-to-position i))
      (push pos res)
      (setq i (1+ i)))
    (nreverse res)))

=> (1 2 2 2 3)

while the correct result is

=> (1 1 2 2 3)

I found that this bug had been noticed before in

http://stackoverflow.com/questions/17588117/emacs-byte-to-position-function-is-not-consistent-with-document

Here's a patch.  The fix may look a bit clumsy but it's actually meant
to avoid pessimizing the presumably common case where the initial
bytepos is at a character boundary.

-- >8 --
Subject: [PATCH] * src/marker.c (buf_bytepos_to_charpos): Fix best_below_byte
 count.

If bytepos is not after a character boundary the preceding loop
overshoots by one character position.
---
 src/marker.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/marker.c b/src/marker.c
index 73928ba..94d676b 100644
--- a/src/marker.c
+++ b/src/marker.c
@@ -341,6 +341,12 @@ buf_bytepos_to_charpos (struct buffer *b, ptrdiff_t bytepos)
 	  BUF_INC_POS (b, best_below_byte);
 	}
 
+      if (best_below_byte != bytepos)
+	{
+	  best_below--;
+	  BUF_DEC_POS (b, best_below_byte);
+	}
+
       /* If this position is quite far from the nearest known position,
 	 cache the correspondence by creating a marker here.
 	 It will last until the next GC.
-- 
2.4.2