Subject: Re: issues with recent doprnt-related changes

From: Eli Zaretskii <[email-masked]>
Date: Thu, 28 Apr 2011 08:39:38 -0400

> Date: Thu, 28 Apr 2011 00:17:01 -0700
> From: Paul Eggert <[email-masked]>
> CC: [email-masked]
> 
> On 04/27/11 22:50, Eli Zaretskii wrote:
> 
> >> * verror might invoke doprnt two or more times, which means that
> >>   doprnt will traverse ap twice.  This does not work in general; the C
> >>   standard is quite clear that the behavior is undefined in this case.
> > 
> > Are there any platforms supported by Emacs where this actually
> > happens? 
> 
> Yes, absolutely.  The bug happens on the platform I normally develop
> Emacs on, namely RHEL 5.6 (x86-64).

What is the implementation of va_start on that platform?  Is stashing
away the ap argument all that's required here?

> > If someone wants to display a 4001-byte
> > message that ends with a multibyte non-ASCII character, let them be
> > punished for not knowing how to write concisely.
> 
> It's not merely a question of the format string itself.
> It could be that there's an error message that formats long file names
> that contain non-ASCII characters, or something else like that;
> the point is that the file names themselves are not under the Emacs
> programmer's control.

If that's what you mean, it's easy to fix.  Done.

> > I'm closing this bug.
> 
> That doesn't sound right.  doprnt has real bugs, which will bite
> users when they process large but plausible strings.  These bugs
> should be fixed.  Could you please reopen the bug report?

You don't need me to reopen a bug, and you don't need an open bug
report to work on some code, if you feel like it.

> > all because of your stubborn insistence on using a library
> > function where it doesn't fit the bill.
> 
> This misrepresents my position.  I already agreed with
> you that it's OK for Emacs to use its own doprnt function
> <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8435#58>,
> and I have not insisted on using any particular library function.
> Please don't personalize this bug report with phrases like
> "your stubborn insistence", and please do not ascribe to me
> motivations that I do not have.

I have no idea what are your motivations.  All I know is what you
write and I read, and how you act.  I see that and I make my
conclusions.  If you don't want your positions misrepresented, try
making your words and deeds speak unequivocally about them.  E.g., try
insisting less that you are always right and others are always wrong,
when judging merits and demerits of alternative solutions to problems.

IOW, this is primarily a social activity.  Good spirit around here can
easily be destroyed by lack of respect to dissenting opinions.  Trying
to reach some kind of consensus, and if that's not possible, some
compromise, is a much better way of having your positions and your
motivations respected.

And that is the last time I'm talking about this in this regard.

> >>  And why is there another test "n * 10 > SIZE_MAX - (fmt[1] -
> >> '0')" that always returns 0, no matter what?
> > 
> > ??? What happens if n*10 is SIZE_MAX-1 and fmt[1] is '2'?
> 
> n * 10 cannot possibly be SIZE_MAX - 1, because at that
> point n < SIZE_MAX / 10, which means n * 10 is at most
> SIZE_MAX - SIZE_MAX % 10 - 10.  fmt[1] is an ASCII digit,
> so n * 10 therefore cannot possibly be greater than
> SIZE_MAX - (fmt[1] - '0').  Hence the expression
> n * 10 > SIZE_MAX - (fmt[1] - '0') always returns zero.

So now you suddenly realize that the ">=" in the first part of that if
clause was a good idea after all, yes?

> >>   /* Avoid int overflow, because many sprintfs seriously mess up
> >>      with widths or precisions greater than INT_MAX.  Avoid size_t
> >>      overflow, since our counters use size_t.  This test is slightly
> >>      conservative, for speed and simplicity.  */
> >>   if (n >= min (INT_MAX, SIZE_MAX) / 10)
> >>      error ("Format width or precision too large");
> > 
> > Sorry, I don't see how this is clearer.
> 
> It's clearer because it says that it's intended that the test is
> conservative

The intent of the test is clear, but its relation to what is done if
the test succeeds is not.

> and that the test might sometimes err on the side of rejecting
> widths or precisions even though they are not too large for the code
> to represent.

Precisely my motivation to include a possibly redundant test which you
so eloquently argue against above.  Can we have some consistency here,
please?  Can you please judge your own arguments like you judge those
of others?

> >> Emacs size fields count the bytes in the string, and does not
> >> count the terminating null byte (which is not part of the string).
> > 
> > String positions are zero-based and extend to
> > include the terminating null character.  See the relevant parts of the
> > display engine code.
> 
> Yes, a position can point to the terminating null character, but if that
> position is N, then the string contains N bytes (not counting the
> terminating null).

The issue is not whether it includes the null or not.  The issue is
the range of values that an EMACS_INT data type must be able to
represent, and the relation of that range to the maximum number of
bytes in a string and to the largest possible string byte position
that Emacs is able to handle.

> Could you please identify a part of the display engine code that is
> going past the terminating null character, so that I can understand
> the situation better?

Start with reseat_to_string and then read (the relevant parts of)
set_iterator_to_next and next_element_from_c_string, for example.