bug#8435: misuse of error ("...%d...", ...) on 64-bit hosts

[Eli Zaretskii]

> Date: Thu, 07 Apr 2011 13:43:39 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 8435@debbugs.gnu.org
> 
> > It also supported %c format for converting a multibyte character to
> > its integer representation.
> 
> This problem should be OK too.  I manually inspected all the places
> that used the %c format.  All but one of them used %c only to convert
> unibyte characters, so they're OK.  The only exception was in
> charset_iso_charset_parameter, and I modified that function to convert
> the multibyte character before passing it as a string to 'error'.

But we are losing a valuable feature this way, I think.  From now on,
any code that needs to use %c for displaying a character codepoint
will need to convert it manually before calling the message functions.

Taking a step back, this issue is about possible bugs when int and
EMACS_INT data types are mixed up in the calls to functions that could
either call doprnt or printf and its ilk.  So I think it would be
better to fix these problems as follows:

  . Introduce a printf format conversion specifier for converting an
    EMACS_INT data type.

  . Fix all the direct and indirect callers of doprnt to use this new
    specifier when the argument is an EMACS_INT.

  . Fix doprnt to avoid overflow when EMACS_INT is a 64-bit type, if
    it could overflow.  (I don't see such a danger, but maybe I
    overlook something.)

You already did the first two.  So I think what my suggestion boils
down to fixing doprnt (if needed) instead of introducing vsnprintf,
and then all the additional problems caused by that introduction will
be gone.

So could you please tell what are the downsides of keeping doprnt
instead of introducing vsnprintf?

> Another issue has come up in further static analysis.  The vsnprintf
> API does not work for strings longer than INT_MAX bytes.  For
> vmessage's use of vsnprintf this is OK, since (for other reasons) a
> frame title can't be that long.  However, for verror this is an
> arbitrary limitation on typical 64-bit hosts.  I'll look into this,
> and plan to propose a further patch to handle it.

I don't think we have any reason to support strings longer than
INT_MAX in these functions.  They are used to display messages in the
echo area/minibuffer, so they can hardly be close to INT_MAX anyway.
We could simply document that and move on.  For bullet-proof code, we
could even check the length and truncate the string before passing it
to verror or its subroutines.

I also don't think we should remove message_nolog, even if it's
currently unused.  It's a useful function.  If someone feels badly
about having dead code, we could #ifdef it away, although I don't
think it matters for such a short function.