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

vim7: Expression with side effects passed to repeated parameter in macro

Expand Messages
  • Walter Briscoe
    After acceptance of my changes of extern to static with PC-lint, I am emboldened to suggest other changes. As far as I know, all the code I am focussing on
    Message 1 of 5 , Jun 2, 2005
    View Source
    • 0 Attachment
      After acceptance of my changes of extern to static with PC-lint, I am
      emboldened to suggest other changes.

      As far as I know, all the code I am focussing on works without problem.

      I follow with two examples of the sort of thing I mean:
      1)
      if (!WHITECHAR(gchar_cursor())

      After macro-processing by the Visual C++ 6.0 preprocessor, that line is:
      if (!(((gchar_cursor()) == ' ' || (gchar_cursor()) == '\t') && ...
      My change eliminates the double gchar_cursor() call.
      The corresponding lines are:
      gc = gchar_cursor();
      if (!(((gc) == ' ' || (gc) == '\t') && ...

      2)
      putc(hist_type2char(type, TRUE), fp);
      After macro-processing by the Visual C++ 6.0 preprocessor, that line is:
      (--(fp)->_cnt >= 0 ? 0xff & (*(fp)->_ptr++ =
      (char)(hist_type2char(type, 1))) : _flsbuf((hist_type2char(type,
      1)),(fp)));

      The corresponding line after my change is:
      fputc(hist_type2char(type, 1), fp);
    • Bram Moolenaar
      ... Thanks. Looks good, I ll include it. -- How To Keep A Healthy Level Of Insanity: 14. Put mosquito netting around your work area. Play a tape of jungle
      Message 2 of 5 , Jun 3, 2005
      View Source
      • 0 Attachment
        Walter Briscoe wrote:

        > After acceptance of my changes of extern to static with PC-lint, I am
        > emboldened to suggest other changes.
        >
        > As far as I know, all the code I am focussing on works without problem.
        >
        > I follow with two examples of the sort of thing I mean:
        > 1)
        > if (!WHITECHAR(gchar_cursor())
        >
        > After macro-processing by the Visual C++ 6.0 preprocessor, that line is:
        > if (!(((gchar_cursor()) == ' ' || (gchar_cursor()) == '\t') && ...
        > My change eliminates the double gchar_cursor() call.
        > The corresponding lines are:
        > gc = gchar_cursor();
        > if (!(((gc) == ' ' || (gc) == '\t') && ...
        >
        > 2)
        > putc(hist_type2char(type, TRUE), fp);
        > After macro-processing by the Visual C++ 6.0 preprocessor, that line is:
        > (--(fp)->_cnt >= 0 ? 0xff & (*(fp)->_ptr++ =
        > (char)(hist_type2char(type, 1))) : _flsbuf((hist_type2char(type,
        > 1)),(fp)));
        >
        > The corresponding line after my change is:
        > fputc(hist_type2char(type, 1), fp);

        Thanks. Looks good, I'll include it.

        --
        How To Keep A Healthy Level Of Insanity:
        14. Put mosquito netting around your work area. Play a tape of jungle
        sounds all day.

        /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
        /// Sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
        \\\ Project leader for A-A-P -- http://www.A-A-P.org ///
        \\\ Buy LOTR 3 and help AIDS victims -- http://ICCF.nl/lotr.html ///
      • Walter Briscoe
        In message of Fri, 3 Jun 2005 12:15:32 in , Bram Moolenaar writes ... [snip] ... Wow! You have
        Message 3 of 5 , Jun 5, 2005
        View Source
        • 0 Attachment
          In message <200506031015.j53AFWkO021336@...> of Fri, 3 Jun
          2005 12:15:32 in , Bram Moolenaar <Bram@...> writes
          >
          >Walter Briscoe wrote:
          [snip]

          >Thanks. Looks good, I'll include it.

          Wow! You have been busy! I just synchronised and noticed a couple of
          things I want to question:

          1) Why not this?
          D:\wfb\vim\bld ) diff -u vim7.net/vim7/src/buffer.c vim70aa/vim7/src
          --- vim7.net/vim7/src/buffer.c Sun Jun 5 15:29:46 2005
          +++ vim70aa/vim7/src/buffer.c Sun Jun 5 13:29:48 2005
          @@ -2450,7 +2450,7 @@
          else
          home_replace(buf, buf->b_fname, NameBuff, MAXPATHL, TRUE);

          - vim_snprintf((char *)IObuff, IOSIZE - 20, "%3d%c%c%c%c%c \"%s\"",
          + len = vim_snprintf((char *)IObuff, IOSIZE - 20, "%3d%c%c%c%c%c \"%s\"",
          buf->b_fnum,
          buf->b_p_bl ? ' ' : 'u',
          buf == curbuf ? '%' :
          @@ -2463,7 +2463,6 @@
          NameBuff);

          /* put "line 999" in column 40 or after the file name */
          - len = STRLEN(IObuff);
          i = 40 - vim_strsize(IObuff);
          do
          {

          D:\wfb\vim\bld ) diff -u vim7.net/vim7/src/mark.c vim70aa/vim7/src
          --- vim7.net/vim7/src/mark.c Sun Jun 5 15:33:54 2005
          +++ vim70aa/vim7/src/mark.c Sun Jun 5 13:37:54 2005
          @@ -1445,7 +1445,7 @@
          char_u *p;
          char_u part[51];
          int retval = FALSE;
          - int n;
          + size_t n;

          name = home_replace_save(NULL, name);
          if (name != NULL)

          D:\wfb\vim\bld )

          2) In removable() in mark.c, I declared a size_t variable which is used
          as foo = STRLEN(part + 1). You changed the type to int. That seems
          strange to me given
          vim.h:#define STRLEN(s) strlen((char *)(s))

          It was REALLY interesting to see the other small changes you made to my
          suggestions. It is just this last one I find strange.
          --
          Walter Briscoe
        • Bram Moolenaar
          ... Yes, the return value of this snprintf() is reliable, we can use it. ... Actually, we can do without n when moving the STRLEN() to inside function call.
          Message 4 of 5 , Jun 5, 2005
          View Source
          • 0 Attachment
            Walter Briscoe wrote:

            > Wow! You have been busy! I just synchronised and noticed a couple of
            > things I want to question:
            >
            > 1) Why not this?
            > D:\wfb\vim\bld ) diff -u vim7.net/vim7/src/buffer.c vim70aa/vim7/src
            > --- vim7.net/vim7/src/buffer.c Sun Jun 5 15:29:46 2005
            > +++ vim70aa/vim7/src/buffer.c Sun Jun 5 13:29:48 2005
            > @@ -2450,7 +2450,7 @@
            > else
            > home_replace(buf, buf->b_fname, NameBuff, MAXPATHL, TRUE);
            >
            > - vim_snprintf((char *)IObuff, IOSIZE - 20, "%3d%c%c%c%c%c \"%s\"",
            > + len = vim_snprintf((char *)IObuff, IOSIZE - 20, "%3d%c%c%c%c%c \"%s\"",
            > buf->b_fnum,
            > buf->b_p_bl ? ' ' : 'u',
            > buf == curbuf ? '%' :
            > @@ -2463,7 +2463,6 @@
            > NameBuff);
            >
            > /* put "line 999" in column 40 or after the file name */
            > - len = STRLEN(IObuff);

            Yes, the return value of this snprintf() is reliable, we can use it.

            > --- vim7.net/vim7/src/mark.c Sun Jun 5 15:33:54 2005
            > +++ vim70aa/vim7/src/mark.c Sun Jun 5 13:37:54 2005
            > @@ -1445,7 +1445,7 @@
            > char_u *p;
            > char_u part[51];
            > int retval = FALSE;
            > - int n;
            > + size_t n;
            >
            > name = home_replace_save(NULL, name);
            > if (name != NULL)

            Actually, we can do without "n" when moving the STRLEN() to inside
            function call.

            --
            To define recursion, we must first define recursion.

            /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
            /// Sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
            \\\ Project leader for A-A-P -- http://www.A-A-P.org ///
            \\\ Buy LOTR 3 and help AIDS victims -- http://ICCF.nl/lotr.html ///
          • Walter Briscoe
            In message of Sun, 5 Jun 2005 21:07:12 in , Bram Moolenaar writes ... [ Near buffer.c(2450), I
            Message 5 of 5 , Jun 6, 2005
            View Source
            • 0 Attachment
              In message <200506051907.j55J7C6j035335@...> of Sun, 5 Jun
              2005 21:07:12 in , Bram Moolenaar <Bram@...> writes
              >
              >Walter Briscoe wrote:

              [ Near buffer.c(2450), I suggested foo=vim_sprintf(bar, ...) better than
              vim_sprintf(bar, ...); foo=STRLEN(bar) ]

              >Yes, the return value of this snprintf() is reliable, we can use it.
              Good!


              [ Near mark.c(1445), I suggested size_t n; is better than int n; given
              the usage of n is n = STRLEN(part + 1)]

              >Actually, we can do without "n" when moving the STRLEN() to inside
              >function call.

              Currently, we have:
              n = STRLEN(part + 1);
              if (MB_STRNICMP(part + 1, name, n) == 0)
              I think moving STRLEN into MB_STRNICMP is a bad idea as:
              1) it is easier to control the current code in a debugger;
              2) Code to call STRLEN is likely to be duplicated in MB_STRNICMP given:
              vim.h:# define MB_STRNICMP(d, s, n) (has_mbyte ? mb_strnicmp((char_u
              *)(d), (char_u *)(s), (int)(n)) : STRNICMP((d), (s), (n)))

              Perhaps a comment about the multiple evaluation should be added.
              I did not know such comments might be needed when suggesting the change.

              Should the FEAT_MBYTE MB_STRNICMP definition map to a function call so
              that the logic complexity is not replicated by each use of MB_STRNICMP?
              --
              Walter Briscoe
            Your message has been successfully submitted and would be delivered to recipients shortly.