Re: [Qemu-devel] [PATCH v9 0/4] Introduce strtosz and make use of it

[Markus Armbruster]

Jes Sorensen <address@hidden> writes:

> On 10/22/10 09:59, Markus Armbruster wrote:
>> address@hidden writes:
>>> From: Jes Sorensen <address@hidden>
>>> This patch introduces cutils.c: strtosz() and gets rid of the
>>> multiple custom hacks for parsing byte sizes. In addition it adds
>>> supports for specifying human style sizes such as 1.5G. Last it
>>> eliminates the horrible abuse of a float to store the byte size for
>>> migrate_set_speed in the monitor.
>>>
>>> Note, this is tested on Linux and build tested for win32 using
>>> mingw32.
>>>
>>> v9: I worked through a couple of revisions directly with Markus and I
>>> think I got it right finally. 
>> 
>> I'd prefer to have strtosz() to match strtol() & friends and not
>> restrict suffixes.  But this code does what it claims to do, as far as I
>> can see, so:
>> 
>> ACK series
>
> Thanks!
>
> I thought about this a fair bit and I believe doing the full test in the
> function is the most valuable. It's a personal preference obviously.
> I think it's a win to do it here since it simplifies the caller code.

But strtosz() doesn't know what suffixes the caller wants accepted.  It
accepting suffixes starting with whitespace, '\0' and ',' is just a
guess.  Three cases:

1. The guess is exactly right.  strtosz()'s checking saves the caller
the check.

2. It's too lose, caller wants to accept fewer suffixes, e.g. just '\0'.
strtosz()'s checking doesn't save the caller anything.

3. It's too tight, caller wants to accept more suffixes.  Caller needs
to make a copy without the suffix.  In the general case, this requires
duplicating strtosz()'s parsing logic.  It's easier not to use it then.

The uses of strtosz() added by this series all fall into case 2.  The
callers lack the check, which means we accept trailing garbage.  Fine
with me, because it's no worse than before.

> Would be great to get this applied so I can get it off my plate :)

Agreed.