Loading ...
Sorry, an error occurred while loading the content.

undo-corruption and valgrind error

Expand Messages
  • Christian Brabandt
    Bram, while investigating tests for makeing ul settings global-local, I see some problems with the undo state. ./vim -u NONE -N -c :set ul=5 1):for i in
    Message 1 of 4 , Nov 6, 2013
    • 0 Attachment
      Bram,
      while investigating tests for makeing 'ul' settings global-local, I see
      some problems with the undo state.

      ./vim -u NONE -N -c ':set ul=5'
      1):for i in range(1,10)| call feedkeys("o".i."\e", 't')|endfor
      (i.e. make sure echo line is a single undoable change).
      2)(undo as long as it is possible, e.g. press u 6 times)
      3)(press o7<esc>)

      Note: undo will be possible exactly once. If you enable the u_check
      functions by e.g. defining U_DEBUG, you'll notice many b_u_newhead and
      b_u_numhead invalid messages at step 3

      Also I see some valgrind error:

      ==7292== 3,357 (568 direct, 2,789 indirect) bytes in 1 blocks are definitely lost in loss record 342 of 363
      ==7292== at 0x4C2A2DB: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
      ==7292== by 0x4DA1D5: lalloc (misc2.c:929)
      ==7292== by 0x5B61FE: u_savecommon (undo.c:430)
      ==7292== by 0x5B604B: u_savedel (undo.c:301)
      ==7292== by 0x4CC7D1: del_lines (misc1.c:2586)
      ==7292== by 0x4FBB4A: op_delete (ops.c:1875)
      ==7292== by 0x471F86: ex_operators (ex_docmd.c:8563)
      ==7292== by 0x46829E: do_one_cmd (ex_docmd.c:2689)
      ==7292== by 0x465831: do_cmdline (ex_docmd.c:1127)
      ==7292== by 0x4F1C47: nv_colon (normal.c:5461)
      ==7292== by 0x4EB1E4: normal_cmd (normal.c:1200)
      ==7292== by 0x5CF277: main_loop (main.c:1329)
      ==7292== by 0x5CEB92: main (main.c:1020)

      (here at step 3 I deleted the whole buffer, that's why you see the
      op_delete function calls).

      The problem is, when freeing the curbuf->b_u_oldhead branch, one loses
      the connection to the newly created curbuf->b_u_newhead header.

      The attached patch fixes it, by making sure, there always exist a link
      between the oldhead header and the newhead header.

      regards,
      Christian
      --

      --
      --
      You received this message from the "vim_dev" maillist.
      Do not top-post! Type your reply below the text you are replying to.
      For more information, visit http://www.vim.org/maillist.php

      ---
      You received this message because you are subscribed to the Google Groups "vim_dev" group.
      To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
      For more options, visit https://groups.google.com/groups/opt_out.
    • Bram Moolenaar
      ... I m glad you found this problem. But your solution is just working around it. The problem is that when deleting the curbuf- b_u_oldhead branch it
      Message 2 of 4 , Nov 6, 2013
      • 0 Attachment
        Christian Brabandt wrote:

        > Bram,
        > while investigating tests for makeing 'ul' settings global-local, I see
        > some problems with the undo state.
        >
        > ./vim -u NONE -N -c ':set ul=5'
        > 1):for i in range(1,10)| call feedkeys("o".i."\e", 't')|endfor
        > (i.e. make sure echo line is a single undoable change).
        > 2)(undo as long as it is possible, e.g. press u 6 times)
        > 3)(press o7<esc>)
        >
        > Note: undo will be possible exactly once. If you enable the u_check
        > functions by e.g. defining U_DEBUG, you'll notice many b_u_newhead and
        > b_u_numhead invalid messages at step 3
        >
        > Also I see some valgrind error:
        >
        > ==7292== 3,357 (568 direct, 2,789 indirect) bytes in 1 blocks are definitely lost in loss record 342 of 363
        > ==7292== at 0x4C2A2DB: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
        > ==7292== by 0x4DA1D5: lalloc (misc2.c:929)
        > ==7292== by 0x5B61FE: u_savecommon (undo.c:430)
        > ==7292== by 0x5B604B: u_savedel (undo.c:301)
        > ==7292== by 0x4CC7D1: del_lines (misc1.c:2586)
        > ==7292== by 0x4FBB4A: op_delete (ops.c:1875)
        > ==7292== by 0x471F86: ex_operators (ex_docmd.c:8563)
        > ==7292== by 0x46829E: do_one_cmd (ex_docmd.c:2689)
        > ==7292== by 0x465831: do_cmdline (ex_docmd.c:1127)
        > ==7292== by 0x4F1C47: nv_colon (normal.c:5461)
        > ==7292== by 0x4EB1E4: normal_cmd (normal.c:1200)
        > ==7292== by 0x5CF277: main_loop (main.c:1329)
        > ==7292== by 0x5CEB92: main (main.c:1020)
        >
        > (here at step 3 I deleted the whole buffer, that's why you see the
        > op_delete function calls).
        >
        > The problem is, when freeing the curbuf->b_u_oldhead branch, one loses
        > the connection to the newly created curbuf->b_u_newhead header.
        >
        > The attached patch fixes it, by making sure, there always exist a link
        > between the oldhead header and the newhead header.

        I'm glad you found this problem. But your solution is just working
        around it. The problem is that when deleting the curbuf->b_u_oldhead
        branch it actually only deletes one entry. It requires a loop to delete
        the whole of the branch.

        I'll make a patch that does it this way. The tests pass, but please
        check if there isn't a problem that the tests don't cover.

        --
        Another bucket of what can only be described as human ordure hits ARTHUR.
        ARTHUR: ... Right! (to the KNIGHTS) That settles it!
        "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

        /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
        /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
        \\\ an exciting new programming language -- http://www.Zimbu.org ///
        \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

        --
        --
        You received this message from the "vim_dev" maillist.
        Do not top-post! Type your reply below the text you are replying to.
        For more information, visit http://www.vim.org/maillist.php

        ---
        You received this message because you are subscribed to the Google Groups "vim_dev" group.
        To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
        For more options, visit https://groups.google.com/groups/opt_out.
      • Christian Brabandt
        ... But that means once the complete undo branch is undone and one adds another change, one loses all undo information? So after step 3, you will only be able
        Message 3 of 4 , Nov 6, 2013
        • 0 Attachment
          On Thu, November 7, 2013 03:04, Bram Moolenaar wrote:
          > Christian Brabandt wrote:
          >> ./vim -u NONE -N -c ':set ul=5'
          >> 1):for i in range(1,10)| call feedkeys("o".i."\e", 't')|endfor
          >> (i.e. make sure echo line is a single undoable change).
          >> 2)(undo as long as it is possible, e.g. press u 6 times)
          >> 3)(press o7<esc>)
          > I'm glad you found this problem. But your solution is just working
          > around it. The problem is that when deleting the curbuf->b_u_oldhead
          > branch it actually only deletes one entry. It requires a loop to delete
          > the whole of the branch.
          >
          > I'll make a patch that does it this way. The tests pass, but please
          > check if there isn't a problem that the tests don't cover.

          But that means once the complete undo branch is undone and one adds
          another change, one loses all undo information? So after step 3, you
          will only be able to undo exactly step 3 and not get further, although
          the undolevels is at 5?

          Well, my attempt wasn't any better. Although my patch preserved the
          undo history, Vim yelled at me with an E438: u_undo: line numbers
          wrong.

          regards,
          Christian

          --
          --
          You received this message from the "vim_dev" maillist.
          Do not top-post! Type your reply below the text you are replying to.
          For more information, visit http://www.vim.org/maillist.php

          ---
          You received this message because you are subscribed to the Google Groups "vim_dev" group.
          To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
          For more options, visit https://groups.google.com/groups/opt_out.
        • Bram Moolenaar
          ... Yes, that s exactly the reason the whole branch is dropped, you can t remove one undo able change and expect the newer ones to work. The only other
          Message 4 of 4 , Nov 7, 2013
          • 0 Attachment
            Christian Brabandt wrote:

            > On Thu, November 7, 2013 03:04, Bram Moolenaar wrote:
            > > Christian Brabandt wrote:
            > >> ./vim -u NONE -N -c ':set ul=5'
            > >> 1):for i in range(1,10)| call feedkeys("o".i."\e", 't')|endfor
            > >> (i.e. make sure echo line is a single undoable change).
            > >> 2)(undo as long as it is possible, e.g. press u 6 times)
            > >> 3)(press o7<esc>)
            > > I'm glad you found this problem. But your solution is just working
            > > around it. The problem is that when deleting the curbuf->b_u_oldhead
            > > branch it actually only deletes one entry. It requires a loop to delete
            > > the whole of the branch.
            > >
            > > I'll make a patch that does it this way. The tests pass, but please
            > > check if there isn't a problem that the tests don't cover.
            >
            > But that means once the complete undo branch is undone and one adds
            > another change, one loses all undo information? So after step 3, you
            > will only be able to undo exactly step 3 and not get further, although
            > the undolevels is at 5?
            >
            > Well, my attempt wasn't any better. Although my patch preserved the
            > undo history, Vim yelled at me with an E438: u_undo: line numbers
            > wrong.

            Yes, that's exactly the reason the whole branch is dropped, you can't
            remove one undo'able change and expect the newer ones to work.

            The only other solution would be to be flexible about 'undolevels', keep
            a few more changes for a short while. Problem is that it's very
            difficult to find the changes to free after that short while.


            --
            Communication is one of the most compli..., eh, well, it's hard.
            You know what I mean. Not?

            /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
            /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
            \\\ an exciting new programming language -- http://www.Zimbu.org ///
            \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

            --
            --
            You received this message from the "vim_dev" maillist.
            Do not top-post! Type your reply below the text you are replying to.
            For more information, visit http://www.vim.org/maillist.php

            ---
            You received this message because you are subscribed to the Google Groups "vim_dev" group.
            To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscribe@....
            For more options, visit https://groups.google.com/groups/opt_out.
          Your message has been successfully submitted and would be delivered to recipients shortly.