From: Eli Zaretskii <eliz@gnu.org>
To: Paul Eggert <eggert@cs.ucla.edu>
Cc: 8545@debbugs.gnu.org
Subject: bug#8545: issues with recent doprnt-related changes
Date: Wed, 27 Apr 2011 22:34:45 +0300 [thread overview]
Message-ID: <83aafb8p4a.fsf@gnu.org> (raw)
In-Reply-To: <4DB65FF1.5010003@cs.ucla.edu>
> Date: Mon, 25 Apr 2011 23:02:25 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 8545@debbugs.gnu.org
>
> On 04/25/11 02:00, Eli Zaretskii wrote:
>
> >> * Format strings never include embedded null bytes, so there's
> >> no need for doprnt to support that.
> >
> > Potentially, someone could call `error' with its first argument taken
> > from a Lisp string, which could include null characters. But again,
> > this feature was there to begin with, and I see no particular need to
> > remove it.
>
> The feature is buggy, because the code does not check
> fmt versus fmt_end every time it increases fmt; it checks
> only sometimes.
I added more checks, thanks for pointing this out.
> "%l" is a strange case anyway, since one cannot reliably use
> "%l" as an alias for "%d". For example, the format "%dx" prints
> an integer followed by an 'x', but if you try to use "%lx" instead,
> it doesn't work. At least, we should remove "%l" as a format
> specifier, as it's a rightly-unused feature and it's just asking
> for trouble to try to support it.
You convinced me, so I removed %l.
> >> * If the format string is too long, the alloca inside doprnt will
> >> crash Emacs on some hosts.
> >
> > You are right. I modified doprnt to use SAFE_ALLOCA instead.
>
> There's no need for alloca or SAFE_ALLOCA or xmalloc or any
> dynamic allocator. Instead, convert any width and precision
> values to integers, and use "*". For example, if the caller
> specifies this:
>
> "%012345.6789g", 3.14
>
> pass this to sprintf:
>
> "%0*.*g", 12345, 6789, 3.14
I see no reason for such complexity, just to avoid SAFE_ALLOCA. But
feel free to make this change, if you think it's important enough.
> >> - doprnt uses atoi (&fmtcpy[1]), but surely this isn't right if
> >> there are flags such as '-'.
> >
> > Why not? In that case, atoi will produce a negative value for
> > `width', which is already handled by the code. If I'm missing
> > something, please point out the specific problems with that.
>
> I don't see how the negative value is handled correctly.
> %-10s means to print a string right-justified, but the code
> surely treats it as if it were %0s.
??? %-10s means to print a string LEFT-justified, and the code handles
that in this loop (which runs after the string was copied to its
destination):
if (minlen < 0)
{
while (minlen < - width && bufsize > 0)
{
*bufptr++ = ' ';
bufsize--;
minlen++;
}
minlen = 0;
}
I actually tried using %-30s, and it did work correctly (as did %30s).
> And other flags
> are possible, e.g., atoi will parse "%0-3d" as if the
> width were zero, but the width is 3 (the "0" is a flag).
The code doesn't call atoi for numeric arguments. It delegates that
case to sprintf, which will handle the likes of %0-3d correctly. And
for %s and %c the "0" flag is not supported anyway (as stated in the
comments) and GCC flags that with a warning. So I see no problem
here.
> A quick second scan found a minor bug in size parsing: the
> expression "n >= SIZE_MAX / 10" should be "n > SIZE_MAX / 10".
When they get to messages as long as SIZE_MAX, let them sue me for
taking away one byte. verror will reject SIZE_MAX-long messages
anyway, so I see no reason to squeeze one more byte here just to throw
it away there.
> /* Limit the string to sizes that both Emacs and size_t can represent. */
> size_t size_max = min (MOST_POSITIVE_FIXNUM + 1, SIZE_MAX);
"MOST_POSITIVE_FIXNUM + 1" is too much, since MOST_POSITIVE_FIXNUM
should be able to cover the terminating null character in Emacs. So I
used this:
size_t size_max = min (MOST_POSITIVE_FIXNUM, SIZE_MAX);
> Thanks, can you make a similar change inside doprint? It
> also uses xrealloc where xfree+xmalloc would be better.
Done.
> One other thing, the documentation says that lower-case l
> is a flag, but it's a length modifer and not a flag.
I fixed the doc on that account.
next prev parent reply other threads:[~2011-04-27 19:34 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-25 5:46 bug#8545: issues with recent doprnt-related changes Paul Eggert
2011-04-25 9:00 ` Eli Zaretskii
2011-04-25 13:37 ` Stefan Monnier
2011-04-26 20:25 ` Paul Eggert
2011-04-27 1:14 ` Stefan Monnier
2011-04-26 6:02 ` Paul Eggert
2011-04-27 19:34 ` Eli Zaretskii [this message]
2011-04-27 23:51 ` Paul Eggert
2011-04-28 1:32 ` Juanma Barranquero
2011-04-28 3:11 ` Paul Eggert
2011-04-28 3:42 ` Juanma Barranquero
2011-04-28 5:06 ` Paul Eggert
2011-04-28 5:15 ` Eli Zaretskii
2011-04-28 5:29 ` Paul Eggert
2011-04-28 6:10 ` Eli Zaretskii
2011-04-28 6:42 ` Paul Eggert
2011-04-28 7:26 ` Eli Zaretskii
2011-04-28 7:54 ` Paul Eggert
2011-04-28 11:14 ` Eli Zaretskii
2011-04-29 12:28 ` Richard Stallman
2011-04-29 19:56 ` Eli Zaretskii
2011-04-29 23:49 ` Paul Eggert
2011-04-30 21:03 ` Richard Stallman
2011-05-01 5:41 ` Paul Eggert
2011-05-01 23:59 ` Richard Stallman
2011-05-02 0:23 ` Paul Eggert
[not found] ` <E1QH37h-0001yM-HR@fencepost.gnu.org>
2011-05-03 20:24 ` Paul Eggert
2011-05-01 4:25 ` Jason Rumney
2011-05-01 5:56 ` Paul Eggert
2011-05-01 8:12 ` Jason Rumney
2011-05-01 11:02 ` Andreas Schwab
2011-04-28 5:02 ` Eli Zaretskii
2011-04-28 5:50 ` Eli Zaretskii
[not found] ` <4DB9146D.2040702@cs.ucla.edu>
[not found] ` <E1QFQVO-0004Dq-6o@fencepost.gnu.org>
[not found] ` <4DB9E5FF.9020506@cs.ucla.edu>
2011-04-29 11:16 ` Eli Zaretskii
2011-04-29 14:41 ` Paul Eggert
2011-04-29 19:35 ` Eli Zaretskii
2011-04-29 20:32 ` Paul Eggert
2011-04-30 8:59 ` Eli Zaretskii
2011-05-04 7:28 ` Paul Eggert
2011-05-04 9:52 ` Eli Zaretskii
2011-05-04 14:56 ` Paul Eggert
2011-05-05 20:36 ` Eli Zaretskii
2011-05-06 13:33 ` bug#8545: " Stefan Monnier
2011-05-06 13:33 ` Stefan Monnier
2011-05-06 14:41 ` bug#8545: " Paul Eggert
2011-05-06 14:41 ` Paul Eggert
2011-05-06 15:03 ` Eli Zaretskii
2011-05-06 17:13 ` Stefan Monnier
2011-05-06 19:57 ` Eli Zaretskii
2011-05-07 3:18 ` Stefan Monnier
2011-05-07 7:55 ` Eli Zaretskii
2011-05-07 7:55 ` bug#8545: " Eli Zaretskii
2011-05-07 3:18 ` Stefan Monnier
2011-05-06 19:57 ` Eli Zaretskii
2011-05-06 17:13 ` Stefan Monnier
2011-05-06 15:03 ` Eli Zaretskii
2011-05-05 20:36 ` Eli Zaretskii
2011-05-04 14:56 ` Paul Eggert
-- strict thread matches above, loose matches on Subject: below --
2011-05-01 18:19 bug#8601: * 2 -> * 4 typo fix in detect_coding_charset Paul Eggert
2011-05-01 19:06 ` Andreas Schwab
2011-05-01 19:25 ` Paul Eggert
2011-05-06 7:29 ` bug#8601: Merged fixes for 8600, 8601, 8602, and (partially) for 8545 Paul Eggert
2020-09-14 12:37 ` bug#8545: " Lars Ingebrigtsen
2020-09-14 18:41 ` Eli Zaretskii
2020-09-16 2:01 ` Paul Eggert
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=83aafb8p4a.fsf@gnu.org \
--to=eliz@gnu.org \
--cc=8545@debbugs.gnu.org \
--cc=eggert@cs.ucla.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.