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

[patch] fixed compilation warnings + memory leak + crash (lesstif)

Expand Messages
  • Dominique Pelle
    After installing libxt-dev lesstif2 lesstif2-dev libxpm-dev libxpm4, I built Vim-7.1.311 ( huge with Lesstif GUI) and found a couple of minor issues: -
    Message 1 of 7 , Jun 6, 2008
      After installing libxt-dev lesstif2 lesstif2-dev libxpm-dev libxpm4,
      I built Vim-7.1.311 ('huge' with Lesstif GUI) and found a couple of
      minor issues:

      - compilation warnings in vim7/src/gui_motif.c
      - crash when exiting Vim & gvim (only when built with -DEXITFREE)
      - memory leak in xsmp_close()


      1/ Compilation warnings in vim7/src/gui_motif.c

      gui_motif.c: In function 'manage_centered':
      gui_motif.c:689: warning: missing sentinel in function call
      gui_motif.c:690: warning: missing sentinel in function call
      gui_motif.c:726: warning: missing sentinel in function call
      gui_motif.c: In function 'do_mnemonic':
      gui_motif.c:1996: warning: missing sentinel in function call
      gui_motif.c:2004: warning: missing sentinel in function call
      gui_motif.c:2011: warning: missing sentinel in function call
      gui_motif.c:2022: warning: missing sentinel in function call
      gui_motif.c: In function 'add_mnemonic_grabs':
      gui_motif.c:2076: warning: missing sentinel in function call
      gui_motif.c:2084: warning: missing sentinel in function call
      gui_motif.c:2091: warning: missing sentinel in function call

      gui_motif.c:

      689 XtVaGetValues(shell, XmNmappedWhenManaged, &mappedWhenManaged, 0);
      690 XtVaSetValues(shell, XmNmappedWhenManaged, False, 0);

      Declaration in /usr/include/X11/Intrinsic.h:

      extern void XtVaGetValues(
      Widget /* widget */,
      ...
      ) _X_SENTINEL(0);

      The sentinel attribute is described in...
      http://gcc.gnu.org/onlinedocs/gcc-4.3.0/gcc/Function-Attributes.html#Function>
      It requires NULL (not 0) as last argument of XtVaGetValues(...).


      2/ Crash (sometimes, not all the time) when exiting Vim or gvim when
      built with -DEXITFREE.

      Valgrind memory checker detects the following error:

      ==9224== Invalid read of size 4
      ==9224== at 0x442AF1B: (within /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x441840A: XtCallCallbackList (in /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x44223FF: (within /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x4421C78: (within /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x4421D71: (within /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x442207C: _XtDoPhase2Destroy (in /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x44221F2: XtDestroyWidget (in /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x4422956: (within /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x4422E36: XtCloseDisplay (in /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x8141F42: mch_free_mem (os_unix.c:2894)
      ==9224== by 0x810BE79: free_all_mem (misc2.c:1125)
      ==9224== by 0x81420E5: mch_exit (os_unix.c:3012)
      ==9224== Address 0x4dad72c is 52 bytes inside a block of size 64 free'd
      ==9224== at 0x402265C: free (vg_replace_malloc.c:323)
      ==9224== by 0x4417D90: XtFree (in /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x44223B8: (within /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x4421C78: (within /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x4421E01: (within /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x442207C: _XtDoPhase2Destroy (in /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x44221F2: XtDestroyWidget (in /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x4422956: (within /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x4422E36: XtCloseDisplay (in /usr/lib/libXt.so.6.0.0)
      ==9224== by 0x8141F42: mch_free_mem (os_unix.c:2894)
      ==9224== by 0x810BE79: free_all_mem (misc2.c:1125)
      ==9224== by 0x81420E5: mch_exit (os_unix.c:3012)
      (more errors follow)

      XtCloseDisplay() accesses freed memory. It looks like a bug
      in X11/Xt/Lesstif.

      I noticed that calls to XtCloseDisplay() & XtDestroyApplication()
      were already commented out in several places with the comment
      'Lesstif and Solaris crash here' in os_unix.c, but not commented
      out yet in mch_free_mem() where I observe the crash. So I
      commented it out just like in other places.


      3/ Memory leak in xsmp_close()

      ==10076== 38 bytes in 1 blocks are definitely lost in loss record 42 of 130
      ==10076== at 0x4022AB8: malloc (vg_replace_malloc.c:207)
      ==10076== by 0x43F1167: _SmcProcessMessage (in /usr/lib/libSM.so.6.0.0)
      ==10076== by 0x4404AB2: IceProcessMessages (in /usr/lib/libICE.so.6.3.0)
      ==10076== by 0x43ED5A2: SmcOpenConnection (in /usr/lib/libSM.so.6.0.0)
      ==10076== by 0x8145F81: xsmp_init (os_unix.c:6596)
      ==10076== by 0x80DE98C: main (main.c:496)

      os_unix.c:

      6595 /* Create an SM connection */
      6596 xsmp.smcconn = SmcOpenConnection(
      6597 NULL,
      6598 NULL,
      6599 SmProtoMajor,
      6600 SmProtoMinor,
      6601 SmcSaveYourselfProcMask | SmcDieProcMask
      6602 | SmcSaveCompleteProcMask |
      SmcShutdownCancelledProcMask,
      6603 &smcallbacks,
      6604 NULL,
      6605 &clientid,
      6606 sizeof(errorstring),
      6607 errorstring);

      SmcOpenConnection() is documented there:

      http://www.cptec.inpe.br/sx4/sx4man2/g1ae04e/chap12.html

      It says:

      If SmcOpenConnection succeeds, the function returns an opaque
      connection pointer of type SmcConn and the client_id_ret argument
      contains the client ID to be used for this session. ***client_id_ret
      should be freed with a call to free() when no longer needed.***

      clientid is currently never freed in os_unix.c (hence leak).
      I observe the leak when exiting, but since xsmp_close() is called
      from other places too, so there may also be leaks even before exiting.

      Attached patches:
      - gui_motif.c.patch
      - os_unix.c.patch

      -- Dominique

      --~--~---------~--~----~------------~-------~--~----~
      You received this message from the "vim_dev" maillist.
      For more information, visit http://www.vim.org/maillist.php
      -~----------~----~----~----~------~----~------~--~---
    • Dominique Pelle
      On Sat, Jun 7, 2008 at 8:18 AM, Dominique Pelle ... In my last patch os_unix.c.patch , I had combined together the fix for the crash and the leak. For the
      Message 2 of 7 , Jun 6, 2008
        On Sat, Jun 7, 2008 at 8:18 AM, Dominique Pelle
        <dominique.pelle@...> wrote:

        > After installing libxt-dev lesstif2 lesstif2-dev libxpm-dev libxpm4,
        > I built Vim-7.1.311 ('huge' with Lesstif GUI) and found a couple of
        > minor issues:
        >
        > - compilation warnings in vim7/src/gui_motif.c
        > - crash when exiting Vim & gvim (only when built with -DEXITFREE)
        > - memory leak in xsmp_close()

        ...snip...

        > Attached patches:
        > - gui_motif.c.patch
        > - os_unix.c.patch


        In my last patch "os_unix.c.patch", I had combined together the fix
        for the crash and the leak. For the sake of clarity, I submit them again
        as 2 separate patches since they are independent issues.

        Attached:
        - crash-os_unix.c.patch
        - leak-os_unix.c.patch

        -- Dominique

        --~--~---------~--~----~------------~-------~--~----~
        You received this message from the "vim_dev" maillist.
        For more information, visit http://www.vim.org/maillist.php
        -~----------~----~----~----~------~----~------~--~---
      • Bram Moolenaar
        ... Thanks for the patches, I ll include them. -- The goal of science is to build better mousetraps. The goal of nature is to build better mice. /// Bram
        Message 3 of 7 , Jun 7, 2008
          Dominique Pelle wrote:

          > After installing libxt-dev lesstif2 lesstif2-dev libxpm-dev libxpm4,
          > I built Vim-7.1.311 ('huge' with Lesstif GUI) and found a couple of
          > minor issues:
          >
          > - compilation warnings in vim7/src/gui_motif.c
          > - crash when exiting Vim & gvim (only when built with -DEXITFREE)
          > - memory leak in xsmp_close()

          Thanks for the patches, I'll include them.

          --
          The goal of science is to build better mousetraps.
          The goal of nature is to build better mice.

          /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
          /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
          \\\ download, build and distribute -- http://www.A-A-P.org ///
          \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

          --~--~---------~--~----~------------~-------~--~----~
          You received this message from the "vim_dev" maillist.
          For more information, visit http://www.vim.org/maillist.php
          -~----------~----~----~----~------~----~------~--~---
        • Dominique Pelle
          ... I built the gui-motif, gui-athena and gui-gtk2 on Linux x86 and the crash in os_unix.c only happens with gui-motif (I m using LessTif). So instead of doing
          Message 4 of 7 , Jun 7, 2008
            On Sat, Jun 7, 2008 at 8:44 AM, Dominique Pelle wrote:

            > Attached:
            > - crash-os_unix.c.patch
            > - leak-os_unix.c.patch

            I built the gui-motif, gui-athena and gui-gtk2 on Linux x86 and the crash in
            os_unix.c only happens with gui-motif (I'm using LessTif).

            So instead of doing #if 0 as in my previous patch, it should be better to
            do #ifndef FEAT_GUI_MOTIF.

            gui-athena shows errors with -DEXITFREE just a few lines below though:

            ==15738== Invalid read of size 4
            ==15738== at 0x42CDEE7: XCloseDisplay (in /usr/lib/libX11.so.6.2.0)
            ==15738== by 0x8140419: mch_free_mem (os_unix.c:2900)
            ==15738== by 0x810A969: free_all_mem (misc2.c:1125)
            ==15738== by 0x8140573: mch_exit (os_unix.c:3012)
            ==15738== by 0x80DE4C8: getout (main.c:1342)
            ==15738== by 0x80A56C3: ex_quit_all (ex_docmd.c:6281)
            ==15738== by 0x809F8CB: do_one_cmd (ex_docmd.c:2623)
            ==15738== by 0x809D18C: do_cmdline (ex_docmd.c:1099)
            ==15738== by 0x812030E: nv_colon (normal.c:5179)
            ==15738== by 0x8119B88: normal_cmd (normal.c:1152)
            ==15738== by 0x80DE20A: main_loop (main.c:1181)
            ==15738== by 0x80DDD5A: main (main.c:940)
            ==15738== Address 0x44020cc is 148 bytes inside a block of size 1,340 free'd
            ==15738== at 0x402265C: free (vg_replace_malloc.c:323)
            ==15738== by 0x42E0F3E: _XFreeDisplayStructure (in /usr/lib/libX11.so.6.2.0)
            ==15738== by 0x42CDFC5: XCloseDisplay (in /usr/lib/libX11.so.6.2.0)
            ==15738== by 0x424EC49: (within /usr/lib/libXt.so.6.0.0)
            ==15738== by 0x424EE36: XtCloseDisplay (in /usr/lib/libXt.so.6.0.0)
            ==15738== by 0x424EE70: (within /usr/lib/libXt.so.6.0.0)
            ==15738== by 0x424F164: XtDestroyApplicationContext (in
            /usr/lib/libXt.so.6.0.0)
            ==15738== by 0x81403F4: mch_free_mem (os_unix.c:2896)
            ==15738== by 0x810A969: free_all_mem (misc2.c:1125)
            ==15738== by 0x8140573: mch_exit (os_unix.c:3012)
            ==15738== by 0x80DE4C8: getout (main.c:1342)
            ==15738== by 0x80A56C3: ex_quit_all (ex_docmd.c:6281)
            ==15738== by 0x809F8CB: do_one_cmd (ex_docmd.c:2623)
            ==15738== by 0x809D18C: do_cmdline (ex_docmd.c:1099)
            ==15738== by 0x812030E: nv_colon (normal.c:5179)
            ==15738== by 0x8119B88: normal_cmd (normal.c:1152)
            ==15738== by 0x80DE20A: main_loop (main.c:1181)
            ==15738== by 0x80DDD5A: main (main.c:940)

            os_unix.c:

            2890 # if (defined(FEAT_X11) && defined(FEAT_XCLIPBOARD)) || defined(PROTO)
            2891 if (xterm_Shell != (Widget)0)
            2892 XtDestroyWidget(xterm_Shell);
            2893 if (xterm_dpy != NULL)
            2894 XtCloseDisplay(xterm_dpy);
            2895 if (app_context != (XtAppContext)NULL)
            !! 2896 XtDestroyApplicationContext(app_context);
            2897 # endif
            2898 # ifdef FEAT_X11
            2899 if (x11_display != NULL && x11_display != xterm_dpy)
            !! 2900 XCloseDisplay(x11_display);
            2901 # endif

            Line 2900 is using memory which was freed at line 2896
            according to valgrind. So it seems that
            XtDestroyApplicationContext(app_context) is also
            destroying x11_display.

            Attached patch fixes the error.

            -- Dominique

            --~--~---------~--~----~------------~-------~--~----~
            You received this message from the "vim_dev" maillist.
            For more information, visit http://www.vim.org/maillist.php
            -~----------~----~----~----~------~----~------~--~---
          • Bram Moolenaar
            ... Thanks. I wonder if we can use #ifndef LESSTIF_VERSION instead of FEAT_GUI_MOTIF. The other #if 0 suggests that Solaris also has this problem. But
            Message 5 of 7 , Jun 8, 2008
              Dominique Pelle wrote:

              > On Sat, Jun 7, 2008 at 8:44 AM, Dominique Pelle wrote:
              >
              > > Attached:
              > > - crash-os_unix.c.patch
              > > - leak-os_unix.c.patch
              >
              > I built the gui-motif, gui-athena and gui-gtk2 on Linux x86 and the crash in
              > os_unix.c only happens with gui-motif (I'm using LessTif).
              >
              > So instead of doing #if 0 as in my previous patch, it should be better to
              > do #ifndef FEAT_GUI_MOTIF.
              >
              > gui-athena shows errors with -DEXITFREE just a few lines below though:
              >
              > ==15738== Invalid read of size 4
              > ==15738== at 0x42CDEE7: XCloseDisplay (in /usr/lib/libX11.so.6.2.0)
              > ==15738== by 0x8140419: mch_free_mem (os_unix.c:2900)
              > ==15738== by 0x810A969: free_all_mem (misc2.c:1125)
              > ==15738== by 0x8140573: mch_exit (os_unix.c:3012)
              > ==15738== by 0x80DE4C8: getout (main.c:1342)
              > ==15738== by 0x80A56C3: ex_quit_all (ex_docmd.c:6281)
              > ==15738== by 0x809F8CB: do_one_cmd (ex_docmd.c:2623)
              > ==15738== by 0x809D18C: do_cmdline (ex_docmd.c:1099)
              > ==15738== by 0x812030E: nv_colon (normal.c:5179)
              > ==15738== by 0x8119B88: normal_cmd (normal.c:1152)
              > ==15738== by 0x80DE20A: main_loop (main.c:1181)
              > ==15738== by 0x80DDD5A: main (main.c:940)
              > ==15738== Address 0x44020cc is 148 bytes inside a block of size 1,340 free'd
              > ==15738== at 0x402265C: free (vg_replace_malloc.c:323)
              > ==15738== by 0x42E0F3E: _XFreeDisplayStructure (in /usr/lib/libX11.so.6.2.0)
              > ==15738== by 0x42CDFC5: XCloseDisplay (in /usr/lib/libX11.so.6.2.0)
              > ==15738== by 0x424EC49: (within /usr/lib/libXt.so.6.0.0)
              > ==15738== by 0x424EE36: XtCloseDisplay (in /usr/lib/libXt.so.6.0.0)
              > ==15738== by 0x424EE70: (within /usr/lib/libXt.so.6.0.0)
              > ==15738== by 0x424F164: XtDestroyApplicationContext (in
              > /usr/lib/libXt.so.6.0.0)
              > ==15738== by 0x81403F4: mch_free_mem (os_unix.c:2896)
              > ==15738== by 0x810A969: free_all_mem (misc2.c:1125)
              > ==15738== by 0x8140573: mch_exit (os_unix.c:3012)
              > ==15738== by 0x80DE4C8: getout (main.c:1342)
              > ==15738== by 0x80A56C3: ex_quit_all (ex_docmd.c:6281)
              > ==15738== by 0x809F8CB: do_one_cmd (ex_docmd.c:2623)
              > ==15738== by 0x809D18C: do_cmdline (ex_docmd.c:1099)
              > ==15738== by 0x812030E: nv_colon (normal.c:5179)
              > ==15738== by 0x8119B88: normal_cmd (normal.c:1152)
              > ==15738== by 0x80DE20A: main_loop (main.c:1181)
              > ==15738== by 0x80DDD5A: main (main.c:940)
              >
              > os_unix.c:
              >
              > 2890 # if (defined(FEAT_X11) && defined(FEAT_XCLIPBOARD)) || defined(PROTO)
              > 2891 if (xterm_Shell != (Widget)0)
              > 2892 XtDestroyWidget(xterm_Shell);
              > 2893 if (xterm_dpy != NULL)
              > 2894 XtCloseDisplay(xterm_dpy);
              > 2895 if (app_context != (XtAppContext)NULL)
              > !! 2896 XtDestroyApplicationContext(app_context);
              > 2897 # endif
              > 2898 # ifdef FEAT_X11
              > 2899 if (x11_display != NULL && x11_display != xterm_dpy)
              > !! 2900 XCloseDisplay(x11_display);
              > 2901 # endif
              >
              > Line 2900 is using memory which was freed at line 2896
              > according to valgrind. So it seems that
              > XtDestroyApplicationContext(app_context) is also
              > destroying x11_display.
              >
              > Attached patch fixes the error.

              Thanks. I wonder if we can use #ifndef LESSTIF_VERSION instead of
              FEAT_GUI_MOTIF. The other "#if 0" suggests that Solaris also has this
              problem. But I'm not sure about this specific code, since it was added
              much later.

              --
              Me? A skeptic? I trust you have proof.

              /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
              /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
              \\\ download, build and distribute -- http://www.A-A-P.org ///
              \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

              --~--~---------~--~----~------------~-------~--~----~
              You received this message from the "vim_dev" maillist.
              For more information, visit http://www.vim.org/maillist.php
              -~----------~----~----~----~------~----~------~--~---
            • Dominique Pelle
              ... Yes, #ifndef LESSTIF_VERSION is better than #ifndef FEAT_GUI_MOTIF. I tested it with both lesstif and Open Motif, it s fine. After the attached patch,
              Message 6 of 7 , Jun 8, 2008
                On Sun, Jun 8, 2008 at 2:25 PM, Bram Moolenaar <Bram@...> wrote:

                > Dominique Pelle wrote:
                >
                >> On Sat, Jun 7, 2008 at 8:44 AM, Dominique Pelle wrote:
                >>
                >> > Attached:
                >> > - crash-os_unix.c.patch
                >> > - leak-os_unix.c.patch
                >>
                >> I built the gui-motif, gui-athena and gui-gtk2 on Linux x86 and the crash in
                >> os_unix.c only happens with gui-motif (I'm using LessTif).
                >>
                >> So instead of doing #if 0 as in my previous patch, it should be better to
                >> do #ifndef FEAT_GUI_MOTIF.
                >>
                >> gui-athena shows errors with -DEXITFREE just a few lines below though:
                >>
                >> ==15738== Invalid read of size 4
                >> ==15738== at 0x42CDEE7: XCloseDisplay (in /usr/lib/libX11.so.6.2.0)
                >> ==15738== by 0x8140419: mch_free_mem (os_unix.c:2900)
                >> ==15738== by 0x810A969: free_all_mem (misc2.c:1125)
                >> ==15738== by 0x8140573: mch_exit (os_unix.c:3012)
                >> ==15738== by 0x80DE4C8: getout (main.c:1342)
                >> ==15738== by 0x80A56C3: ex_quit_all (ex_docmd.c:6281)
                >> ==15738== by 0x809F8CB: do_one_cmd (ex_docmd.c:2623)
                >> ==15738== by 0x809D18C: do_cmdline (ex_docmd.c:1099)
                >> ==15738== by 0x812030E: nv_colon (normal.c:5179)
                >> ==15738== by 0x8119B88: normal_cmd (normal.c:1152)
                >> ==15738== by 0x80DE20A: main_loop (main.c:1181)
                >> ==15738== by 0x80DDD5A: main (main.c:940)
                >> ==15738== Address 0x44020cc is 148 bytes inside a block of size 1,340 free'd
                >> ==15738== at 0x402265C: free (vg_replace_malloc.c:323)
                >> ==15738== by 0x42E0F3E: _XFreeDisplayStructure (in /usr/lib/libX11.so.6.2.0)
                >> ==15738== by 0x42CDFC5: XCloseDisplay (in /usr/lib/libX11.so.6.2.0)
                >> ==15738== by 0x424EC49: (within /usr/lib/libXt.so.6.0.0)
                >> ==15738== by 0x424EE36: XtCloseDisplay (in /usr/lib/libXt.so.6.0.0)
                >> ==15738== by 0x424EE70: (within /usr/lib/libXt.so.6.0.0)
                >> ==15738== by 0x424F164: XtDestroyApplicationContext (in
                >> /usr/lib/libXt.so.6.0.0)
                >> ==15738== by 0x81403F4: mch_free_mem (os_unix.c:2896)
                >> ==15738== by 0x810A969: free_all_mem (misc2.c:1125)
                >> ==15738== by 0x8140573: mch_exit (os_unix.c:3012)
                >> ==15738== by 0x80DE4C8: getout (main.c:1342)
                >> ==15738== by 0x80A56C3: ex_quit_all (ex_docmd.c:6281)
                >> ==15738== by 0x809F8CB: do_one_cmd (ex_docmd.c:2623)
                >> ==15738== by 0x809D18C: do_cmdline (ex_docmd.c:1099)
                >> ==15738== by 0x812030E: nv_colon (normal.c:5179)
                >> ==15738== by 0x8119B88: normal_cmd (normal.c:1152)
                >> ==15738== by 0x80DE20A: main_loop (main.c:1181)
                >> ==15738== by 0x80DDD5A: main (main.c:940)
                >>
                >> os_unix.c:
                >>
                >> 2890 # if (defined(FEAT_X11) && defined(FEAT_XCLIPBOARD)) || defined(PROTO)
                >> 2891 if (xterm_Shell != (Widget)0)
                >> 2892 XtDestroyWidget(xterm_Shell);
                >> 2893 if (xterm_dpy != NULL)
                >> 2894 XtCloseDisplay(xterm_dpy);
                >> 2895 if (app_context != (XtAppContext)NULL)
                >> !! 2896 XtDestroyApplicationContext(app_context);
                >> 2897 # endif
                >> 2898 # ifdef FEAT_X11
                >> 2899 if (x11_display != NULL && x11_display != xterm_dpy)
                >> !! 2900 XCloseDisplay(x11_display);
                >> 2901 # endif
                >>
                >> Line 2900 is using memory which was freed at line 2896
                >> according to valgrind. So it seems that
                >> XtDestroyApplicationContext(app_context) is also
                >> destroying x11_display.
                >>
                >> Attached patch fixes the error.
                >
                > Thanks. I wonder if we can use #ifndef LESSTIF_VERSION instead of
                > FEAT_GUI_MOTIF. The other "#if 0" suggests that Solaris also has this
                > problem. But I'm not sure about this specific code, since it was added
                > much later.


                Yes, #ifndef LESSTIF_VERSION is better than #ifndef FEAT_GUI_MOTIF.
                I tested it with both lesstif and Open Motif, it's fine. After the attached
                patch, neither lesstif, nor Open Motif access freed memory.

                Open Motif needs the same fix as for athena gui i.e. set x11_display
                to NULL after 'XtDestroyApplicationContext(app_context);' since it
                seems to take care of already calling 'XCloseDisplay(x11_display)'.

                Lesstif and Open Motif also use uninitialized memory but I assume
                that's a problem in those libs and not with Vim.

                There are several things not freed with all GUIs (athena, open motif
                lesstif, gtk2, gnome2) when exiting even with -DEXITFREE but that's
                not a major problem I think. I fixed at least the following leak which
                happens at least for Motif and athena gui (patch: leak-gui_x11.c).

                ==23613== 32 bytes in 1 blocks are definitely lost in loss record 34 of 126
                ==23613== at 0x4022AB8: malloc (vg_replace_malloc.c:207)
                ==23613== by 0x40A8C08: xpmParseDataAndCreate (in /usr/lib/libXm.so.2.0.1)
                ==23613== by 0x40AA0A6: XpmCreateImageFromData (in /usr/lib/libXm.so.2.0.1)
                ==23613== by 0x40AA342: XpmCreatePixmapFromData (in /usr/lib/libXm.so.2.0.1)
                ==23613== by 0x81C9E0E: gui_mch_init (gui_x11.c:1541)
                ==23613== by 0x81BA6F1: gui_init (gui.c:457)
                ==23613== by 0x81A56BD: set_termname (term.c:1845)
                ==23613== by 0x81A609D: termcapinit (term.c:2514)
                ==23613== by 0x81BA0BE: gui_start (gui.c:91)
                ==23613== by 0x80DED45: main (main.c:634)

                -- Dominique

                --~--~---------~--~----~------------~-------~--~----~
                You received this message from the "vim_dev" maillist.
                For more information, visit http://www.vim.org/maillist.php
                -~----------~----~----~----~------~----~------~--~---
              • Bram Moolenaar
                ... Yes, it s tough fixing all memory leaks for GUIs. The documentation isn t always clear about when memory can be freed. I ll include the ones the patched,
                Message 7 of 7 , Jun 8, 2008
                  Dominique Pelle wrote:

                  > On Sun, Jun 8, 2008 at 2:25 PM, Bram Moolenaar <Bram@...> wrote:
                  >
                  > > Dominique Pelle wrote:
                  > >
                  > >> On Sat, Jun 7, 2008 at 8:44 AM, Dominique Pelle wrote:
                  > >>
                  > >> > Attached:
                  > >> > - crash-os_unix.c.patch
                  > >> > - leak-os_unix.c.patch
                  > >>
                  > >> I built the gui-motif, gui-athena and gui-gtk2 on Linux x86 and the crash in
                  > >> os_unix.c only happens with gui-motif (I'm using LessTif).
                  > >>
                  > >> So instead of doing #if 0 as in my previous patch, it should be better to
                  > >> do #ifndef FEAT_GUI_MOTIF.
                  > >>
                  > >> gui-athena shows errors with -DEXITFREE just a few lines below though:
                  > >>
                  > >> ==15738== Invalid read of size 4
                  > >> ==15738== at 0x42CDEE7: XCloseDisplay (in /usr/lib/libX11.so.6.2.0)
                  > >> ==15738== by 0x8140419: mch_free_mem (os_unix.c:2900)
                  > >> ==15738== by 0x810A969: free_all_mem (misc2.c:1125)
                  > >> ==15738== by 0x8140573: mch_exit (os_unix.c:3012)
                  > >> ==15738== by 0x80DE4C8: getout (main.c:1342)
                  > >> ==15738== by 0x80A56C3: ex_quit_all (ex_docmd.c:6281)
                  > >> ==15738== by 0x809F8CB: do_one_cmd (ex_docmd.c:2623)
                  > >> ==15738== by 0x809D18C: do_cmdline (ex_docmd.c:1099)
                  > >> ==15738== by 0x812030E: nv_colon (normal.c:5179)
                  > >> ==15738== by 0x8119B88: normal_cmd (normal.c:1152)
                  > >> ==15738== by 0x80DE20A: main_loop (main.c:1181)
                  > >> ==15738== by 0x80DDD5A: main (main.c:940)
                  > >> ==15738== Address 0x44020cc is 148 bytes inside a block of size 1,340 free'd
                  > >> ==15738== at 0x402265C: free (vg_replace_malloc.c:323)
                  > >> ==15738== by 0x42E0F3E: _XFreeDisplayStructure (in /usr/lib/libX11.so.6.2.0)
                  > >> ==15738== by 0x42CDFC5: XCloseDisplay (in /usr/lib/libX11.so.6.2.0)
                  > >> ==15738== by 0x424EC49: (within /usr/lib/libXt.so.6.0.0)
                  > >> ==15738== by 0x424EE36: XtCloseDisplay (in /usr/lib/libXt.so.6.0.0)
                  > >> ==15738== by 0x424EE70: (within /usr/lib/libXt.so.6.0.0)
                  > >> ==15738== by 0x424F164: XtDestroyApplicationContext (in
                  > >> /usr/lib/libXt.so.6.0.0)
                  > >> ==15738== by 0x81403F4: mch_free_mem (os_unix.c:2896)
                  > >> ==15738== by 0x810A969: free_all_mem (misc2.c:1125)
                  > >> ==15738== by 0x8140573: mch_exit (os_unix.c:3012)
                  > >> ==15738== by 0x80DE4C8: getout (main.c:1342)
                  > >> ==15738== by 0x80A56C3: ex_quit_all (ex_docmd.c:6281)
                  > >> ==15738== by 0x809F8CB: do_one_cmd (ex_docmd.c:2623)
                  > >> ==15738== by 0x809D18C: do_cmdline (ex_docmd.c:1099)
                  > >> ==15738== by 0x812030E: nv_colon (normal.c:5179)
                  > >> ==15738== by 0x8119B88: normal_cmd (normal.c:1152)
                  > >> ==15738== by 0x80DE20A: main_loop (main.c:1181)
                  > >> ==15738== by 0x80DDD5A: main (main.c:940)
                  > >>
                  > >> os_unix.c:
                  > >>
                  > >> 2890 # if (defined(FEAT_X11) && defined(FEAT_XCLIPBOARD)) || defined(PROTO)
                  > >> 2891 if (xterm_Shell != (Widget)0)
                  > >> 2892 XtDestroyWidget(xterm_Shell);
                  > >> 2893 if (xterm_dpy != NULL)
                  > >> 2894 XtCloseDisplay(xterm_dpy);
                  > >> 2895 if (app_context != (XtAppContext)NULL)
                  > >> !! 2896 XtDestroyApplicationContext(app_context);
                  > >> 2897 # endif
                  > >> 2898 # ifdef FEAT_X11
                  > >> 2899 if (x11_display != NULL && x11_display != xterm_dpy)
                  > >> !! 2900 XCloseDisplay(x11_display);
                  > >> 2901 # endif
                  > >>
                  > >> Line 2900 is using memory which was freed at line 2896
                  > >> according to valgrind. So it seems that
                  > >> XtDestroyApplicationContext(app_context) is also
                  > >> destroying x11_display.
                  > >>
                  > >> Attached patch fixes the error.
                  > >
                  > > Thanks. I wonder if we can use #ifndef LESSTIF_VERSION instead of
                  > > FEAT_GUI_MOTIF. The other "#if 0" suggests that Solaris also has this
                  > > problem. But I'm not sure about this specific code, since it was added
                  > > much later.
                  >
                  >
                  > Yes, #ifndef LESSTIF_VERSION is better than #ifndef FEAT_GUI_MOTIF.
                  > I tested it with both lesstif and Open Motif, it's fine. After the attached
                  > patch, neither lesstif, nor Open Motif access freed memory.
                  >
                  > Open Motif needs the same fix as for athena gui i.e. set x11_display
                  > to NULL after 'XtDestroyApplicationContext(app_context);' since it
                  > seems to take care of already calling 'XCloseDisplay(x11_display)'.
                  >
                  > Lesstif and Open Motif also use uninitialized memory but I assume
                  > that's a problem in those libs and not with Vim.
                  >
                  > There are several things not freed with all GUIs (athena, open motif
                  > lesstif, gtk2, gnome2) when exiting even with -DEXITFREE but that's
                  > not a major problem I think. I fixed at least the following leak which
                  > happens at least for Motif and athena gui (patch: leak-gui_x11.c).
                  >
                  > ==23613== 32 bytes in 1 blocks are definitely lost in loss record 34 of 126
                  > ==23613== at 0x4022AB8: malloc (vg_replace_malloc.c:207)
                  > ==23613== by 0x40A8C08: xpmParseDataAndCreate (in /usr/lib/libXm.so.2.0.1)
                  > ==23613== by 0x40AA0A6: XpmCreateImageFromData (in /usr/lib/libXm.so.2.0.1)
                  > ==23613== by 0x40AA342: XpmCreatePixmapFromData (in /usr/lib/libXm.so.2.0.1)
                  > ==23613== by 0x81C9E0E: gui_mch_init (gui_x11.c:1541)
                  > ==23613== by 0x81BA6F1: gui_init (gui.c:457)
                  > ==23613== by 0x81A56BD: set_termname (term.c:1845)
                  > ==23613== by 0x81A609D: termcapinit (term.c:2514)
                  > ==23613== by 0x81BA0BE: gui_start (gui.c:91)
                  > ==23613== by 0x80DED45: main (main.c:634)

                  Yes, it's tough fixing all memory leaks for GUIs. The documentation
                  isn't always clear about when memory can be freed. I'll include the
                  ones the patched, thanks.

                  --
                  hundred-and-one symptoms of being an internet addict:
                  14. You start introducing yourself as "Jim at I-I-Net dot net dot au"

                  /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
                  /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
                  \\\ download, build and distribute -- http://www.A-A-P.org ///
                  \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

                  --~--~---------~--~----~------------~-------~--~----~
                  You received this message from the "vim_dev" maillist.
                  For more information, visit http://www.vim.org/maillist.php
                  -~----------~----~----~----~------~----~------~--~---
                Your message has been successfully submitted and would be delivered to recipients shortly.