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

Re: Patch 7.3.715

Expand Messages
  • Christian Brabandt
    Hi Bram! ... It is still not right so. I ll look into it later. regards, Christian -- -- You received this message from the vim_dev maillist. Do not
    Message 1 of 16 , Nov 23, 2012
      Hi Bram!

      On Fr, 23 Nov 2012, Bram Moolenaar wrote:

      >
      > Christian Brabandt wrote:
      >
      > > Hi Dominique!
      > >
      > > On Do, 22 Nov 2012, Dominique Pellé wrote:
      > >
      > > > I can't tell whether that's the right fix, but I confirm that
      > > > your proposed change avoids the crash at least.
      > > >
      > > > wp->w_llist was set to NULL at line quickfix.c:914 which
      > > > was introduced by chane 3918 according to hg annotate:
      > > >
      > > > 911 vimboss 644: if (qi->qf_listcount == LISTCOUNT)
      > > > 912 vimboss 7: {
      > > > 913 bram 3918: if (wp != NULL && wp->w_llist == qi)
      > > > 914 bram 3918: wp->w_llist = NULL;
      > > > 915 vimboss 644: qf_free(qi, 0);
      > > >
      > > > ===
      > > > changeset: 3918:4f0ddf4137ee
      > > > tag: v7-3-715
      > > > user: Bram Moolenaar <bram@...>
      > > > date: Wed Nov 14 22:38:08 2012 +0100
      > > > files: src/quickfix.c src/testdir/test49.ok
      > > > src/testdir/test49.vim src/version.c
      > > > description:
      > > > updated for version 7.3.715
      > > > Problem: Crash when calling setloclist() in BufUnload autocmd. (Marcin
      > > > Szamotulski)
      > > > Solution: Set w_llist to NULL when it was freed. Also add a test.
      > > > (Christian Brabandt)
      > > > ===
      > > >
      > > > I find it odd that a function called qf_new_list() clears
      > > > wp->w_llist (set it to NULL) and does not set it back
      > > > to something else. The name of the function "qf_new_list()"
      > > > suggests that it should create another list, so perhaps
      > > > it should set wp->w_llist to something else. But I don't
      > > > understand the code here.
      > >
      > > Indeed. I think GET_LOC_LIST should be defined as
      > > ll_get_or_alloc_list(wp)
      >
      > ll_get_or_alloc_list() can still return NULL, thus your check is needed
      > anyway.

      It is still not right so. I'll look into it later.

      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
    • Ike Devolder
      ... I have tested the patch on top of 725 had one segfault already (could not yet reproduce) but the following i can reproduce every time: same configuration
      Message 2 of 16 , Nov 23, 2012
        Op vrijdag 23 november 2012 21:47:47 schreef Christian Brabandt:
        > Hi Bram!
        >
        > On Fr, 23 Nov 2012, Bram Moolenaar wrote:
        > > Christian Brabandt wrote:
        > > > Hi Dominique!
        > > >
        > > > On Do, 22 Nov 2012, Dominique Pellé wrote:
        > > > > I can't tell whether that's the right fix, but I confirm that
        > > > > your proposed change avoids the crash at least.
        > > > >
        > > > > wp->w_llist was set to NULL at line quickfix.c:914 which
        > > > >
        > > > > was introduced by chane 3918 according to hg annotate:
        > > > > 911 vimboss 644: if (qi->qf_listcount == LISTCOUNT)
        > > > > 912 vimboss 7: {
        > > > > 913 bram 3918: if (wp != NULL && wp->w_llist == qi)
        > > > > 914 bram 3918: wp->w_llist = NULL;
        > > > > 915 vimboss 644: qf_free(qi, 0);
        > > > >
        > > > > ===
        > > > > changeset: 3918:4f0ddf4137ee
        > > > > tag: v7-3-715
        > > > > user: Bram Moolenaar <bram@...>
        > > > > date: Wed Nov 14 22:38:08 2012 +0100
        > > > > files: src/quickfix.c src/testdir/test49.ok
        > > > > src/testdir/test49.vim src/version.c
        > > > > description:
        > > > > updated for version 7.3.715
        > > > > Problem: Crash when calling setloclist() in BufUnload autocmd.
        > > > > (Marcin
        > > > >
        > > > > Szamotulski)
        > > > >
        > > > > Solution: Set w_llist to NULL when it was freed. Also add a test.
        > > > >
        > > > > (Christian Brabandt)
        > > > >
        > > > > ===
        > > > >
        > > > > I find it odd that a function called qf_new_list() clears
        > > > > wp->w_llist (set it to NULL) and does not set it back
        > > > > to something else. The name of the function "qf_new_list()"
        > > > > suggests that it should create another list, so perhaps
        > > > > it should set wp->w_llist to something else. But I don't
        > > > > understand the code here.
        > > >
        > > > Indeed. I think GET_LOC_LIST should be defined as
        > > > ll_get_or_alloc_list(wp)
        > >
        > > ll_get_or_alloc_list() can still return NULL, thus your check is needed
        > > anyway.
        >
        > It is still not right so. I'll look into it later.
        >
        > regards,
        > Christian

        I have tested the patch on top of 725
        had one segfault already (could not yet reproduce)

        but the following i can reproduce every time:

        same configuration as before, same file (index.php)
        - open index.php (it still has the syntax error
        - :w (syntastic kicks in)
        - fix the error by adding ; after phpinfo()
        - :w (no more error in the file)
        - remove the ; again
        - :w (syntastic kicks in)
        - :w ->E776: No location list

        "index.php" 2L, 16C written
        Error detected while processing function
        <SNR>31_UpdateErrors..<SNR>31_AutoToggleLocList..<SNR>31_ShowLocList:
        line 3:
        E776: No location list
        Press ENTER or type command to continue

        no crash in this case, if i continue after this issue i get a second quickfix
        buffer.

        thx for the intermediate fix it is definatly an improvement.

        --Ike

        --
        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
        Hi everybody ... The following patch seems to fix all the issues mentioned so far. It basically does this by reverting Patch 7.3.715 and making sure, no data
        Message 3 of 16 , Nov 25, 2012
          Hi everybody

          On Fr, 23 Nov 2012, Ike Devolder wrote:
          > but the following i can reproduce every time:
          >
          > same configuration as before, same file (index.php)
          > - open index.php (it still has the syntax error
          > - :w (syntastic kicks in)
          > - fix the error by adding ; after phpinfo()
          > - :w (no more error in the file)
          > - remove the ; again
          > - :w (syntastic kicks in)
          > - :w ->E776: No location list
          >
          > "index.php" 2L, 16C written
          > Error detected while processing function
          > <SNR>31_UpdateErrors..<SNR>31_AutoToggleLocList..<SNR>31_ShowLocList:
          > line 3:
          > E776: No location list
          > Press ENTER or type command to continue
          >
          > no crash in this case, if i continue after this issue i get a second quickfix
          > buffer.
          >
          > thx for the intermediate fix it is definatly an improvement.

          The following patch seems to fix all the issues mentioned so far. It
          basically does this by reverting Patch 7.3.715 and making sure, no data
          is freed more than once. Unfortunately, I have not been able to come up
          with a simple test case for the syntastic problem, that could be
          included.

          If anybody can come up with such a test (using no plugin, but just using
          vim -u NONE -U NONE -N) a tip is appreciated.

          regards,
          Christian
          --
          Ich bin geldgierig. Als Finanzminister muß man geldgierig sein.
          -- Hans Eichel

          --
          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 for the patch. Have you tried running with valgrind, checking that no memory is leaked or accessing already freed memory? It s only manual
          Message 4 of 16 , Nov 25, 2012
            Christian Brabandt wrote:

            > Hi everybody
            >
            > On Fr, 23 Nov 2012, Ike Devolder wrote:
            > > but the following i can reproduce every time:
            > >
            > > same configuration as before, same file (index.php)
            > > - open index.php (it still has the syntax error
            > > - :w (syntastic kicks in)
            > > - fix the error by adding ; after phpinfo()
            > > - :w (no more error in the file)
            > > - remove the ; again
            > > - :w (syntastic kicks in)
            > > - :w ->E776: No location list
            > >
            > > "index.php" 2L, 16C written
            > > Error detected while processing function
            > > <SNR>31_UpdateErrors..<SNR>31_AutoToggleLocList..<SNR>31_ShowLocList:
            > > line 3:
            > > E776: No location list
            > > Press ENTER or type command to continue
            > >
            > > no crash in this case, if i continue after this issue i get a second quickfix
            > > buffer.
            > >
            > > thx for the intermediate fix it is definatly an improvement.
            >
            > The following patch seems to fix all the issues mentioned so far. It
            > basically does this by reverting Patch 7.3.715 and making sure, no data
            > is freed more than once. Unfortunately, I have not been able to come up
            > with a simple test case for the syntastic problem, that could be
            > included.
            >
            > If anybody can come up with such a test (using no plugin, but just using
            > vim -u NONE -U NONE -N) a tip is appreciated.

            Thanks for the patch. Have you tried running with valgrind, checking
            that no memory is leaked or accessing already freed memory? It's only
            manual testing, better than nothing.


            --
            hundred-and-one symptoms of being an internet addict:
            73. You give your dog used motherboards instead of bones

            /// 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
          • Ike Devolder
            ... thx all seems to be working fine with this patch -- Ike -- You received this message from the vim_dev maillist. Do not top-post! Type your reply below
            Message 5 of 16 , Nov 25, 2012
              On Sun, Nov 25, 2012 at 02:29:59PM +0100, Christian Brabandt wrote:
              > Hi everybody
              >
              > On Fr, 23 Nov 2012, Ike Devolder wrote:
              > > but the following i can reproduce every time:
              > >
              > > same configuration as before, same file (index.php)
              > > - open index.php (it still has the syntax error
              > > - :w (syntastic kicks in)
              > > - fix the error by adding ; after phpinfo()
              > > - :w (no more error in the file)
              > > - remove the ; again
              > > - :w (syntastic kicks in)
              > > - :w ->E776: No location list
              > >
              > > "index.php" 2L, 16C written
              > > Error detected while processing function
              > > <SNR>31_UpdateErrors..<SNR>31_AutoToggleLocList..<SNR>31_ShowLocList:
              > > line 3:
              > > E776: No location list
              > > Press ENTER or type command to continue
              > >
              > > no crash in this case, if i continue after this issue i get a second quickfix
              > > buffer.
              > >
              > > thx for the intermediate fix it is definatly an improvement.
              >
              > The following patch seems to fix all the issues mentioned so far. It
              > basically does this by reverting Patch 7.3.715 and making sure, no data
              > is freed more than once. Unfortunately, I have not been able to come up
              > with a simple test case for the syntastic problem, that could be
              > included.
              >
              > If anybody can come up with such a test (using no plugin, but just using
              > vim -u NONE -U NONE -N) a tip is appreciated.
              >
              > regards,
              > Christian
              > --
              > Ich bin geldgierig. Als Finanzminister muß man geldgierig sein.
              > -- Hans Eichel
              >
              > --
              > 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

              > diff --git a/src/quickfix.c b/src/quickfix.c
              > --- a/src/quickfix.c
              > +++ b/src/quickfix.c
              > @@ -898,11 +898,7 @@
              > * way with ":grep'.
              > */
              > while (qi->qf_listcount > qi->qf_curlist + 1)
              > - {
              > - if (wp != NULL && wp->w_llist == qi)
              > - wp->w_llist = NULL;
              > qf_free(qi, --qi->qf_listcount);
              > - }
              >
              > /*
              > * When the stack is full, remove to oldest entry
              > @@ -910,8 +906,6 @@
              > */
              > if (qi->qf_listcount == LISTCOUNT)
              > {
              > - if (wp != NULL && wp->w_llist == qi)
              > - wp->w_llist = NULL;
              > qf_free(qi, 0);
              > for (i = 1; i < LISTCOUNT; ++i)
              > qi->qf_lists[i - 1] = qi->qf_lists[i];
              > @@ -2135,13 +2129,17 @@
              > while (qi->qf_lists[idx].qf_count)
              > {
              > qfp = qi->qf_lists[idx].qf_start->qf_next;
              > - 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);
              > + 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;
              > }
              > - vim_free(qi->qf_lists[idx].qf_title);
              > + if (qi->qf_lists[idx].qf_title != NULL)
              > + vim_free(qi->qf_lists[idx].qf_title);
              > qi->qf_lists[idx].qf_title = NULL;
              > }
              >

              thx all seems to be working fine with this patch

              --
              Ike

              --
              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.