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

Useless error check

Expand Messages
  • Ben Schmidt
    Hi, Bram, It came up in a private conversation I had that this line in get_varp() is useless: default: EMSG(_( E356: get_varp ERROR )); The reason is
    Message 1 of 3 , Feb 3, 2011
    View Source
    • 0 Attachment
      Hi, Bram,

      It came up in a private conversation I had that this line in get_varp() is useless:

      default: EMSG(_("E356: get_varp ERROR"));

      The reason is that the first time it is called, it is because set_options_default
      is running at initialisation time, and since this happens in alphabetical order,
      almost every option is initialised before 'verbosefile' is.

      But EMSG eventually checks 'verbosefile' (p_vfile) to write to it, and in so
      doing, it segfaults (in redir_write).

      The patch below fixes it. Don't know if it's worth the bother, though.

      Ben.



      diff -r 4ad7ccf65de3 src/option.c
      --- a/src/option.c Sun Jan 30 08:39:51 2011 +0100
      +++ b/src/option.c Fri Feb 04 03:35:56 2011 +1100
      @@ -3493,6 +3493,10 @@
      tabpage_T *tp;
      #endif

      + /* Initialise p_vfile ('verbosefile') manually first, as if other
      + * initialisations cause errors, it is needed to display them. */
      + p_vfile = (char_u *)"";
      +
      for (i = 0; !istermoption(&options[i]); i++)
      if (!(options[i].flags & P_NODEFAULT))
      set_option_default(i, opt_flags, p_cp);



      --
      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
      ... With this solution other calls to EMSG() too early would still fail. Perhaps we can better check if p_vfile is NULL? And make sure it s NULL to start
      Message 2 of 3 , Feb 3, 2011
      View Source
      • 0 Attachment
        Ben Schmidt wrote:

        > It came up in a private conversation I had that this line in
        > get_varp() is useless:
        >
        > default: EMSG(_("E356: get_varp ERROR"));
        >
        > The reason is that the first time it is called, it is because
        > set_options_default is running at initialisation time, and since this
        > happens in alphabetical order, almost every option is initialised
        > before 'verbosefile' is.
        >
        > But EMSG eventually checks 'verbosefile' (p_vfile) to write to it, and
        > in so doing, it segfaults (in redir_write).
        >
        > The patch below fixes it. Don't know if it's worth the bother, though.

        With this solution other calls to EMSG() too early would still fail.
        Perhaps we can better check if p_vfile is NULL? And make sure it's NULL
        to start with.

        So many things happen startup that it's difficult to avoid mistakes...

        --
        hundred-and-one symptoms of being an internet addict:
        190. You quickly hand over your wallet, leather jacket, and car keys
        during a mugging, then proceed to beat the crap out of your
        assailant when he asks for your laptop.

        /// 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
      • Ben Schmidt
        ... Yeah. It just moves the fence a little bit.... ... Yeah, that works too, of course. Minimal patch below. I don t know if the other places in message.c that
        Message 3 of 3 , Feb 3, 2011
        View Source
        • 0 Attachment
          On 4/02/11 8:03 AM, Bram Moolenaar wrote:
          > Ben Schmidt wrote:
          >
          >> It came up in a private conversation I had that this line in
          >> get_varp() is useless:
          >>
          >> default: EMSG(_("E356: get_varp ERROR"));
          >>
          >> The reason is that the first time it is called, it is because
          >> set_options_default is running at initialisation time, and since this
          >> happens in alphabetical order, almost every option is initialised
          >> before 'verbosefile' is.
          >>
          >> But EMSG eventually checks 'verbosefile' (p_vfile) to write to it, and
          >> in so doing, it segfaults (in redir_write).
          >>
          >> The patch below fixes it. Don't know if it's worth the bother, though.
          >
          > With this solution other calls to EMSG() too early would still fail.

          Yeah. It just moves the fence a little bit....

          > Perhaps we can better check if p_vfile is NULL? And make sure it's NULL
          > to start with.

          Yeah, that works too, of course. Minimal patch below. I don't know if
          the other places in message.c that use p_vfile should be checked, too,
          but I don't really think so. There are also a bunch of references to
          p_vfile in ex_eval.c which I haven't touched, but I have even less idea
          whether they might need it. The only other references are in option.c
          and should be OK.

          Another possibility would be just setting it to a static empty string.
          I've included a patch below for that solution, too. Actually, I think
          that may be the best one.

          I've given them both a quick test (and the earlier one) and they all
          solve the problem I was encountering.

          > So many things happen startup that it's difficult to avoid mistakes...

          Yes. Very. Startup is a scary time. :-) Fortunately, at startup the user
          hasn't done any useful work to lose, though.

          Ben.



          diff -r d80acedce311 src/message.c
          --- a/src/message.c Thu Feb 03 16:08:23 2011 +0000
          +++ b/src/message.c Fri Feb 04 10:51:24 2011 +1100
          @@ -3019,7 +3019,7 @@
          * If 'verbosefile' is set write message in that file.
          * Must come before the rest because of updating "msg_col".
          */
          - if (*p_vfile != NUL)
          + if (p_vfile != NUL && *p_vfile != NUL)
          verbose_write(s, maxlen);

          if (redirecting())
          diff -r d80acedce311 src/option.h
          --- a/src/option.h Thu Feb 03 16:08:23 2011 +0000
          +++ b/src/option.h Fri Feb 04 10:51:24 2011 +1100
          @@ -843,6 +843,10 @@
          #endif
          EXTERN long p_verbose; /* 'verbose' */
          EXTERN char_u *p_vfile; /* 'verbosefile' */
          +#ifdef IN_OPTION_C
          +/* Ensure p_vfile is NUL so that startup errors don't segfault */
          +char_u *p_vfile = NUL; /* 'verbosefile' */
          +#endif
          EXTERN int p_warn; /* 'warn' */
          #ifdef FEAT_CMDL_COMPL
          EXTERN char_u *p_wop; /* 'wildoptions' */



          diff -r d80acedce311 src/option.h
          --- a/src/option.h Thu Feb 03 16:08:23 2011 +0000
          +++ b/src/option.h Fri Feb 04 10:59:14 2011 +1100
          @@ -843,6 +843,10 @@
          #endif
          EXTERN long p_verbose; /* 'verbose' */
          EXTERN char_u *p_vfile; /* 'verbosefile' */
          +#ifdef IN_OPTION_C
          +/* Ensure p_vfile is initialised so that startup errors don't segfault */
          +char_u *p_vfile = (char_u *)""; /* 'verbosefile' */
          +#endif
          EXTERN int p_warn; /* 'warn' */
          #ifdef FEAT_CMDL_COMPL
          EXTERN char_u *p_wop; /* 'wildoptions' */

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