emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Suspicious warning in W64 build


From: Eli Zaretskii
Subject: Re: Suspicious warning in W64 build
Date: Sun, 17 Sep 2017 17:31:00 +0300

> Cc: address@hidden, address@hidden
> From: Paul Eggert <address@hidden>
> Date: Sun, 17 Sep 2017 00:01:09 -0700
> 
> Why was the attached patch needed? What warning did it suppress?

I wrote about that in

  http://lists.gnu.org/archive/html/emacs-devel/2017-09/msg00448.html

The warning is this:

  ../../emacs/src/data.c: In function 'minmax_driver':
  ../../emacs/src/data.c:3022:9: warning: 'accum.i' may be used uninitialized 
in this function [-Wmaybe-uninitialized]
  return accum;
  ^~~~~

Which seems to mean that even eassume is sometimes not enough to
convince GCC 7 that the code is correct:

  static Lisp_Object
  minmax_driver (ptrdiff_t nargs, Lisp_Object *args,
                 enum Arith_Comparison comparison)
  {
    eassume (0 < nargs);  <<<<<<<<<<<<<<<<<<<<<<<
    Lisp_Object accum;
    for (ptrdiff_t argnum = 0; argnum < nargs; argnum++)
      {
        Lisp_Object val = args[argnum];
        CHECK_NUMBER_OR_FLOAT_COERCE_MARKER (val);
        if (argnum == 0 || !NILP (arithcompare (val, accum, comparison)))
          accum = val;
        else if (FLOATP (accum) && isnan (XFLOAT_DATA (accum)))
          return accum;
      }
    return accum;
  }

Since nargs > 0, the loop is always entered, but GCC seems to miss that.

> As you may recall, I prefer using UNINIT to suppress uninitialized-variable 
> warnings because it is more automatable (e.g., it is easier to automate 
> removing it later once GCC gets fixed). If UNINIT does not work I would like 
> to know why.

UNINIT looked inappropriate to initialize a Lisp_Object which is
supposed to be a number.  Even initializing with Qnil didn't look
right to me (it could raise some brows), and using UNINIT is only
equivalent to Qnil under the current representation of Qnil, and so is
insufficiently future-proof.  E.g., it could become an invalid Lisp
object if we decide to change representation at some future point.  By
contrast, args[0] was available, of the right data type, and the
comment I added makes it clear what was the reason for the
initialization.

So using args[0] sounds like a better solution here than UNINIT.  In
any case, it's a valid solution.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]