bug in vi's undo

[Kartik Agaram]

$ git log |head -n1
commit 0972c7f7a570c38edb68e1c60a45614b7a7c7d55
$ uname -a
Linux akkartik 4.4.0-112-generic #135-Ubuntu SMP Fri Jan 19 11:48:14
UTC 2018 i686 i686 i686 GNU/Linux
$ gcc --version
gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609

To reproduce:

$ make menuconfig
# accept all defaults, save config
$ make
$ cat > x
a
b
c
d
e
$ ./busybox vi x
Navigate to the bottom line, delete using 'dd'.
Hit 'dd' again to delete the new bottom line.
Hit 'u' to undo the previous deletion.
Hit 'u' a second time.

Expected contents:
a
b
c
d
e

Contents seen:
^@a
b
c
d

An earlier version also segfaulted at this point.

=== Attempt #1

The issue seems to be fixed by deleting this special-case when the
deleted text is at the end of the buffer:

@@ -2307,10 +2307,6 @@ static void undo_push(char *src, unsigned int
length, uint8_t u_type)    // Add to
        // Allocate a new undo object
        if (u_type == UNDO_DEL || u_type == UNDO_DEL_CHAIN) {
                // For UNDO_DEL objects, save deleted text
-               if ((src + length) == end)
-                       length--;
-               // If this deletion empties text[], strip the newline.
When the buffer becomes
-               // zero-length, a newline is added back, which
requires this to compensate.
                undo_entry = xzalloc(offsetof(struct undo_object,
undo_text) + length);
                memcpy(undo_entry->undo_text, src, length);
        } else {

However, as that deleted comment hints, then I reintroduce a bug in a
different situation:

Delete all 5 lines with '5dd'.
Hit 'u' to undo.

Instead of seeing the original 5 lines, I end up with an extra final newline.

=== Attempt #2

I can fix both situations by changing the condition to match (my
interpretation of) the comment, triggering only when the entire buffer
is deleted:

@@ -2307,7 +2307,7 @@ static void undo_push(char *src, unsigned int
length, uint8_t u_type)     // Add to
        // Allocate a new undo object
        if (u_type == UNDO_DEL || u_type == UNDO_DEL_CHAIN) {
                // For UNDO_DEL objects, save deleted text
-               if ((src + length) == end)
+               if (src == text && (src + length) == end)
                        length--;
                // If this deletion empties text[], strip the newline.
When the buffer becomes
                // zero-length, a newline is added back, which
requires this to compensate.

Can any of you[1] think other situations where this would break?

Thanks,
Kartik

[1] Particularly Jody Bruchon, original author of commit a8d6f9be in
Apr 2014. Thanks in advance, Jody.