|
Message-ID: <20230602150745.GR4163@brightrain.aerifal.cx> 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.