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

Patch 7.3.746

Expand Messages
  • Bram Moolenaar
    Patch 7.3.746 Problem: Memory leaks when using location lists. Solution: Set qf_title to something. (Christian Brabandt) Files: src/eval.c,
    Message 1 of 3 , Dec 5, 2012
    • 0 Attachment
      Patch 7.3.746
      Problem: Memory leaks when using location lists.
      Solution: Set qf_title to something. (Christian Brabandt)
      Files: src/eval.c, src/quickfix.c


      *** ../vim-7.3.745/src/eval.c 2012-10-21 23:55:59.000000000 +0200
      --- src/eval.c 2012-12-05 14:47:56.000000000 +0100
      ***************
      *** 16292,16298 ****
      action = *act;
      }

      ! if (l != NULL && set_errorlist(wp, l, action, NULL) == OK)
      rettv->vval.v_number = 0;
      }
      #endif
      --- 16292,16299 ----
      action = *act;
      }

      ! if (l != NULL && set_errorlist(wp, l, action,
      ! (char_u *)(wp == NULL ? "setqflist()" : "setloclist()")) == OK)
      rettv->vval.v_number = 0;
      }
      #endif
      *** ../vim-7.3.745/src/quickfix.c 2012-11-28 22:12:40.000000000 +0100
      --- src/quickfix.c 2012-12-05 14:51:52.000000000 +0100
      ***************
      *** 2124,2138 ****
      int idx;
      {
      qfline_T *qfp;

      while (qi->qf_lists[idx].qf_count)
      {
      qfp = qi->qf_lists[idx].qf_start->qf_next;
      ! if (qi->qf_lists[idx].qf_title != NULL)
      {
      vim_free(qi->qf_lists[idx].qf_start->qf_text);
      vim_free(qi->qf_lists[idx].qf_start->qf_pattern);
      vim_free(qi->qf_lists[idx].qf_start);
      }
      qi->qf_lists[idx].qf_start = qfp;
      --qi->qf_lists[idx].qf_count;
      --- 2124,2145 ----
      int idx;
      {
      qfline_T *qfp;
      + int stop = FALSE;

      while (qi->qf_lists[idx].qf_count)
      {
      qfp = qi->qf_lists[idx].qf_start->qf_next;
      ! if (qi->qf_lists[idx].qf_title != NULL && !stop)
      {
      vim_free(qi->qf_lists[idx].qf_start->qf_text);
      + stop = (qi->qf_lists[idx].qf_start == qfp);
      vim_free(qi->qf_lists[idx].qf_start->qf_pattern);
      vim_free(qi->qf_lists[idx].qf_start);
      + if (stop)
      + /* Somehow qf_count may have an incorrect value, set it to 1
      + * to avoid crashing when it's wrong.
      + * TODO: Avoid qf_count being incorrect. */
      + qi->qf_lists[idx].qf_count = 1;
      }
      qi->qf_lists[idx].qf_start = qfp;
      --qi->qf_lists[idx].qf_count;
      *** ../vim-7.3.745/src/version.c 2012-12-05 14:42:56.000000000 +0100
      --- src/version.c 2012-12-05 15:15:45.000000000 +0100
      ***************
      *** 727,728 ****
      --- 727,730 ----
      { /* Add new patch number below this line */
      + /**/
      + 746,
      /**/

      --
      hundred-and-one symptoms of being an internet addict:
      98. The Alta Vista administrators ask you what sites are missing
      in their index files.

      /// 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
    • Christian Brabandt
      ... […] ... I finally found the root cause of having qf_count being wrong. The vimgrep doesn t expect, that the quickfix list changes between loading files
      Message 2 of 3 , Dec 6, 2012
      • 0 Attachment
        On Wed, December 5, 2012 15:17, Bram Moolenaar wrote:
        > Patch 7.3.746
        […]
        > + /* Somehow qf_count may have an incorrect value, set it to 1
        > + * to avoid crashing when it's wrong.
        > + * TODO: Avoid qf_count being incorrect. */
        > + qi->qf_lists[idx].qf_count = 1;

        I finally found the root cause of having qf_count being wrong.

        The vimgrep doesn't expect, that the quickfix list changes between
        loading files and uses qf_add_entry() to add entries, which happily
        increments qf_count of the wrong curlist, while the actual entries
        will always be attached at the correct position to which prevp points
        to.

        I think, this patch prevents it, by checking that qi->curlist is
        always the one, vimgrep expects. But please check. I am not entirely
        sure this works correctly in all situations, but at least my tests
        have all been successful so far (valgrind did not report errors, the
        test suite runs as expected and the crashes are avoided)

        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
      • Bram Moolenaar
        ... Thanks. I ll add it near the top of the todo list. -- hundred-and-one symptoms of being an internet addict: 120. You ask a friend, What s that big shiny
        Message 3 of 3 , Dec 7, 2012
        • 0 Attachment
          Christian Brabandt wrote:

          > On Wed, December 5, 2012 15:17, Bram Moolenaar wrote:
          > > Patch 7.3.746
          > […]
          > > + /* Somehow qf_count may have an incorrect value, set it to 1
          > > + * to avoid crashing when it's wrong.
          > > + * TODO: Avoid qf_count being incorrect. */
          > > + qi->qf_lists[idx].qf_count = 1;
          >
          > I finally found the root cause of having qf_count being wrong.
          >
          > The vimgrep doesn't expect, that the quickfix list changes between
          > loading files and uses qf_add_entry() to add entries, which happily
          > increments qf_count of the wrong curlist, while the actual entries
          > will always be attached at the correct position to which prevp points
          > to.
          >
          > I think, this patch prevents it, by checking that qi->curlist is
          > always the one, vimgrep expects. But please check. I am not entirely
          > sure this works correctly in all situations, but at least my tests
          > have all been successful so far (valgrind did not report errors, the
          > test suite runs as expected and the crashes are avoided)

          Thanks. I'll add it near the top of the todo list.

          --
          hundred-and-one symptoms of being an internet addict:
          120. You ask a friend, "What's that big shiny thing?" He says, "It's the sun."

          /// 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
        Your message has been successfully submitted and would be delivered to recipients shortly.