bug#8545: issues with recent doprnt-related changes

[Eli Zaretskii]

> Date: Thu, 28 Apr 2011 15:11:11 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 8545-done@debbugs.gnu.org
> 
> > If that's what you mean, it's easy to fix.  Done.
> 
> If I understand things correctly, that fix (in bzr 104036) handles the
> case where the format itself has a non-ASCII character that is
> truncated, but it doesn't handle the case where the format is
> something like "file name = %s", and %s expands to a long file name
> that is truncated.  If so, surely that case still needs to be fixed.

Ah, yes.  Missed one more place where this truncation could happen.
Should be fixed now (including an older bug with that code).

> Yes, portable code is supposed to use va_copy.  Code that traverses
> through an argument list N times can call va_start once, va_copy N - 1
> times, and va_end N times (once on the original, once on each copy).
> 
> va_copy is a C99-ism, but we can use it as-is in Emacs source code,
> and use the relevant gnulib module for obsolete platforms that lack it.
> Do the DOS and NT ports have va_copy?  If not, it should be simple
> to supply a substitute.

The MS-DOS and MinGW builds use GCC, so they have va_copy by
definition.  MSVC doesn't, but we can provide a trivial definition
which will work for x86.  If we still support MSVC by the time Emacs
can be built as a 64-bit executable on Windows, and if MSVC still
doesn't have va_copy by that time, we can handle this better at that
time.

> Another possibility is to remove the 'if' test entirely, making it the
> caller's responsibility to not specify outlandish widths in format
> strings.

I don't think this is a good idea.  verror is in many cases the last
line of defense, so it should IMO be rock-stable and try very hard to
emit something useful even in the most improbable situations.

For that reason, I also don't like the calls to `abort' you
introduced.  I understand the motivation (detection of invalid Emacs
code), but why not make it call `error' instead, like we do here:

          switch (*fmt++)
            {
            default:
              error ("Invalid format operation %%%s%c",
                     "ll" + 2 - long_flag, fmt[-1]);

After all, using %ll when the long long data type isn't supported is
not different from using %a or some other unsupported format letter,
right?

> OK, thanks.  I read the code, and if I understand it correctly, since
> 'point' is 1-origin, a buffer with MOST_POSITIVE_FIXNUM characters
> will have values of 'point' ranging from 1 through
> MOST_POSITIVE_FIXNUM + 1, but that "+ 1" would mean Fpoint wouldn't
> work: so we should limit buffers to contain at most
> MOST_POSITIVE_FIXNUM - 1 bytes.

I guess so, yes.  I would like to have other opinions, though, so I
will start a new thread on emacs-devel about that.

> Is it also the case that Emacs should limit strings to at most
> MOST_POSITIVE_FIXNUM - 1 bytes?

Only if we are thinking about copying a MOST_POSITIVE_FIXNUM-long
string into a buffer.  Otherwise, since string positions are
zero-based, I think strings are safe with MOST_POSITIVE_FIXNUM.

> Sorry, I couldn't tell this from the functions you mentioned;
> there's a lot of code there, and this stuff isn't immediately
> obvious.

Yeah, tell me about that.  I've been hacking that code extensively for
the last year and a half, and I still don't always know who's who ;-)