Re: PATCH: parse from even deeper in hell

[Mikael Magnusson <mikachu@gmail.com>]

On Mon, Feb 23, 2015 at 2:38 PM, Mikael Magnusson <mikachu@gmail.com> wrote:
> On Mon, Feb 23, 2015 at 1:57 PM, Peter Stephenson
> <p.stephenson@samsung.com> wrote:
>> On Mon, 23 Feb 2015 12:36:07 +0000
>> Peter Stephenson <p.stephenson@samsung.com> wrote:
>>> On Mon, 23 Feb 2015 12:35:51 +0100
>>> Mikael Magnusson <mikachu@gmail.com> wrote:
>>> > I figured out that when we assign lineptr after fiddling with it, we
>>> > also need to update start, it records the location of *lineptr on
>>> > entry to the function, and is used to calculate things later on. With
>>> > that addition, the unmetafy + metafy mostly works. insert-last-word
>>> > still gets "stuck" on words that came from the old metafication and
>>> > starts over from the end of history, leaving the old word in place.
>>>
>>> What's the line this happens with?  Is it the same as the one that
>>> originally failed?
>>
>> Yes, I see it is.
>
> If you start dev-0 and run
> echo た
> then you'll end up with a trouble-causing entry in your history file.
> Simply holding down alt-. in a new dev-1 shell then just inserts a
> bunch of た forever for me.
>
>>> I wonder if it's getting information by some other path, though it seems
>>> strange the last word wouldn't come from the bit we've apparently fixed
>>> up.
>
> At least the function calling this function seems to store the raw
> string in lasthist.text, but that's outside the while loop, so I think
> only for the final history entry? I get my problem even when the bad
> string is in an earlier line.
>
> Oh, there's also this line,
> he->node.nam = ztrdup(pt);
> (pt is the history line)
>
> Do we want to move the whole thing before that? I didn't look too
> closely at this function yet.

Doing so fixes the issue for me.

gmail-mangled patch follows, I also deleted the paragraph about
non-uselex since it's now outside that function.


diff --git i/Src/hist.c w/Src/hist.c
index ee55431..ae255d6 100644
--- i/Src/hist.c
+++ w/Src/hist.c
@@ -2502,11 +2502,42 @@ readhistfile(char *fn, int err, int readflags)
         newflags |= HIST_MAKEUNIQUE;
     while (fpos = ftell(in), (l = readhistline(0, &buf, &bufsiz, in))) {
         char *pt = buf;
+        char *ptr;
+        int remeta;

         if (l < 0) {
         zerr("corrupt history file %s", fn);
         break;
         }
+
+        /*
+         * Handle the special case that we're reading from an
+         * old shell with fewer meta characters, so we need to
+         * metafy some more.  (It's not clear why the history
+         * file is metafied at all; some would say this is plain
+         * stupid.  But we're stuck with it now without some
+         * hairy workarounds for compatibility).
+         *
+         * This is rare so doesn't need to be that efficient; just
+         * allocate space off the heap.
+         */
+        for (ptr = pt; *ptr; ptr++) {
+        if (*ptr == Meta && ptr[1])
+            ptr++;
+        else if (imeta(*ptr)) {
+            remeta = 1;
+            break;
+        }
+        }
+        if (remeta) {
+        unmetafy(pt, &remeta);
+        pt = metafy(pt, remeta, META_USEHEAP);
+        }
+
         if (*pt == ':') {
         pt++;
         stim = zstrtol(pt, NULL, 0);
@@ -3375,37 +3406,7 @@ histsplitwords(char *lineptr, short
 {
     int nwords = *nwordsp, nwordpos = 0, remeta = 0;
     short *words = *wordsp;
-    char *start, *ptr;
-
-    /*
-     * Handle the special case that we're reading from an
-     * old shell with fewer meta characters, so we need to
-     * metafy some more.  (It's not clear why the history
-     * file is metafied at all; some would say this is plain
-     * stupid.  But we're stuck with it now without some
-     * hairy workarounds for compatibility).
-     *
-     * This is rare so doesn't need to be that efficient; just
-     * allocate space off the heap.
-     *
-     * Note that our it's currently believed this all comes out in
-     * the wash in the non-uselex case owing to where unmetafication
-     * and metafication happen.
-     */
-    for (ptr = lineptr; *ptr; ptr++) {
-    if (*ptr == Meta && ptr[1])
-        ptr++;
-    else if (imeta(*ptr)) {
-        remeta = 1;
-        break;
-    }
-    }
-    if (remeta) {
-    unmetafy(lineptr, &remeta);
-    lineptr = metafy(lineptr, remeta, META_USEHEAP);
-    }
-
-    start = lineptr;
+    char *start = lineptr;

     if (uselex) {
     LinkList wordlist;


-- 
Mikael Magnusson