[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Fri, 2 Jun 2023 11:07:45 -0400
From: Rich Felker <dalias@...c.org>
To: Jₑₙₛ Gustedt <jens.gustedt@...ia.fr>
Cc: musl@...ts.openwall.com
Subject: Re: [C23 printf 2/3] C23: implement the wN length specifiers
for printf
On Fri, Jun 02, 2023 at 04:55:47PM +0200, Jₑₙₛ Gustedt wrote:
> Rich,
>
> on Fri, 2 Jun 2023 10:29:37 -0400 you (Rich Felker <dalias@...c.org>)
> wrote:
>
> > ...
>
> > OK, some numbers. This alt version (with a couple bug fixes and
> > support for wf added), vs the full state machine version. On i386,
> > using this instead of the full state machine (which is still missing
> > 'wf') adds 246 bytes of .text but saves 440 bytes of .rodata. With wf
> > added, it will be even more of a win. So I think it makes more sense
> > to go with this version.
>
> Ok, sounds promissing. Some remarks
>
> - I'd still prefer to name the constant `WPRE` instead of `PLAIN` for
> documentation reasons.
Well it's not used for all w prefixes, only for w32/wf32/wf16. It's
really a state for "we already encountered some explicit prefix that
means plain int". I could call it W32PRE or something but that seems
misleading or at least incomplete too...?
> - It seems that your version doesn't capture the leading 0 case. That
> would still need an `if` condition that leads to `invalid`, I
> guess.
It only enters the processing
if (*s=='w' && s[1]-'1'<9u)
which is false for w0...
The subsequent automaton then rejects the unknown 'w'.
In the fixed up version with wf support, I separated the 2 conditions
(and fixed the failure to advance s before passing it to getint), so
it now just has inside the 'w' processing block:
if (*++s-'1'>=9u) goto inval;
I'll send the full v2 shortly.
> > As an aside, since think the state table is the same for wide printf
> > as for normal, and since wide printf already depends on normal printf,
> > we could make the states table external (but hidden, of course) and
> > let wide printf reuse it, for some decent size savings and elimination
> > of source-level redundancy (DRY).
>
> Same holds for the `pop_arg` function, which is also repeated.
Nice find, thanks! Deduplicating this requires also moving the union
type to a place it can be shared, but this is reasonable I think and
gives a place to declare the functions too.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
