[PATCH] unify itoa

[Rob Landley]

On Saturday 08 July 2006 2:42 pm, Denis Vlasenko wrote:
> On Saturday 08 July 2006 19:45, Rob Landley wrote:
> > On Saturday 08 July 2006 12:44 pm, Denis Vlasenko wrote:
> > > Hi folks, Rob,
> > >
> > > Attached patch removes all instances of local itoa()-like
> > > functions, introduces common one into libbb and uses
> > > it where appropriate.
> >
> > Is "sprintf" really that bad?  (Or xasprintf() out of our library when
> > you need it to malloc?)  Especially if the darn compiler actually starts
> > doing intelligent string merging, which it's inching towards...
>
> I think that some programs may print so much numerical output
> that it may be important for them to do itoa more-or-less
> efficiently.

Could you give me an example?  (Your patch didn't actually convert many users, 
that I saw.)

> sprintf is a performance disaster. In any libc I've looked into,
> it _creates a custom struct FILE_, and then calls v_something_printf()
> on that, which _parses format string_ with gazzilion of possible
> obscure format specifiers, and _then_ it does itoa thing.

That would be a yes, then. :)

> > You have a large quantity of code under #ifdef NOT_NEEDED_YET that by
> > itself would make me reject this patch.
>
> I wrote that just in case you'll want me to convert ash too.
> See below.

I'm far more interested in replacing ash than fixing it, and am working 
towards that end here (albeit by clearing a couple other low hanging fruit 
off my todo list first so I can focus on bbsh with fewer interruptions).

> > (What's max_unsigned_power10() doing,
> > anyway, and is that really a sane way to do whatever it is?)
>
> It calculates maximum power-of-10 which fits in unsigned int.
> It does that at compile time, portably. That's why it looks funny.
>
> gcc optimizes it out to simple 1000000000 constant
> (if sizeof(unsigned)==4, that is).

When is sizeof(unsigned) not going to be 4 on any Linux platform?  We use the 
lp64 model (as do all modern Unix variants, including MacOS X), where only 
long and pointer are 64 bit.

http://www.unix.org/whitepapers/64bit.html

This even works for Windows, which uses llp64 because they decided to make 64 
bit programming really inconvenient in the name of backwards compatability, 
and still managed to to screw it up:

http://blogs.msdn.com/oldnewthing/archive/2005/01/31/363790.aspx

Not that I care about that part. :)

> > > If you want it converted too, I'll do it. itoa.c
> > > already has code which can handle unsigned long long conv,
> > > it is #ifdef'ed out for now. See the patch.
> >
> > It can handle int, and long long, but not long.  On 64 bit platforms (we
> > already support x86-64, and this sort of thing will only get more common
> > to the point it may be a noticeable chunk of the embedded space in 3-5
> > years), these are three different sizes.
>
> long long currently is needed for ash only, everybody else wants
> itoa([unsigned] int). There are no ltoa users today.
> ltoa is of course a piece of cake, can do when someone will need it.
>
> So, do you want me to
>
> a) remove #ifdef NOT_NEEDED_YET part
> 	or
> b) enable it and use it in ash?

The first, please.

A general rule of thumb: we can always add code to the tree later.  (And if we 
remove it, we can always add it _back_ later.)

Thanks,

Rob
-- 
Never bet against the cheap plastic solution.