Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
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.