Date: Thu, 11 Jul 2019 10:34:01 -0700 From: John Baldwin <jhb@FreeBSD.org> To: Phil Shafer <phil@juniper.net> Cc: arch@FreeBSD.org Subject: Re: expand_number(3) Message-ID: <79c3ce4d-0f4b-37f3-9e66-58a71d67962c@FreeBSD.org> In-Reply-To: <F947286A-9D2A-48A8-9C30-76879D6A7A0E@juniper.net> References: <afdc443d-3138-baca-337a-1111c6df4f7c@FreeBSD.org> <F947286A-9D2A-48A8-9C30-76879D6A7A0E@juniper.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On 7/10/19 2:29 PM, Phil Shafer wrote: > Would a reasonable goal be to make expand_number and humanize_number > (with HN_IEC_PREFIXES) should be as compatible as possible? In regards to IEC, I think mapping 'K' to 1000 by default would not be great, but we should support '2MiB' in addition to '2MB' probably. > The API doesn't allow many humanize-style features, but having the > number->string->number work would be ideal so support for > HN_DECIMAL-style input should be added, so "1.1K" does the right thing > (thought it rounds to 1126). Hmm, that seems harder to reason about, or maybe it requires parsing the original value as a float or double. I'm tempted to only support whole numbers to give more deterministic behavior such that if expand_number does support a value you will always get what you passed to humanize_number. > Lacking support for "suffix", trailing garbage should not be an error > (ala atoi), though I wish there was a parameter like strtol()'s endptr > to pass the next point to the character back in. If there was a suffix > parameter, the endptr would be skipped over it, if present. Yes, the API doesn't really make it easy to deal with trailing garbage as in some cases (e.g. bhyve's memory size) trailing garbage is an error and unless expand_number detects it I have no way of enforcing that. I don't really like atoi() for that reason even though it is convenient. I'm not sure of a good answer here. > The expand_number API uses uint64_t where humanize_number uses signed > int64_t so the answer for negative numbers is DOA. Except you can cast the result and it works (see the tests in the review). That said, I'm fine with that being the case, we should just pick one and document it as such. > Is it worth making a new expandize_number() function > (dehumanize_number?) that brings the API more into sync with > humanize_number? The main thing I wish was some way to reason about trailing garbage, but for now I'd just like to more clearly document how the API should work. > Thanks, > Phil > > > > On 10 Jul 2019, at 14:14, John Baldwin wrote: > >> While working on some bhyve changes I happened to look at the source >> to expand_number(3) and noticed what I think are several bugs. I >> wrote some tests to validate that these bugs are present, but in some >> cases I'm not sure what the correct / desired behavior is? I posted >> a review with the tests and questions about what some of the behavior >> should be with folks who have worked on expand_number(3) previously, >> but haven't gotten any feedback. The URL to the review is below if >> other folks would like to offer feedback on some of my open questions >> such as should we support negative numbers, hex and octal bases, >> trailing garbage, etc.? >> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.freebsd.org_D20796&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=And7spKEXmRNIrq8pYCiSg&m=EHG6EmocNCDS6VoOZVxzZoCq-BqbJjBpsC9kXhGFTd0&s=bUQ8xvhg6vv95W5fHxswOrzDIeVxFsFJIa2582tjHgA&e= >> >> -- >> John Baldwin >> _______________________________________________ >> freebsd-arch@freebsd.org mailing list >> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freebsd.org_mailman_listinfo_freebsd-2Darch&d=DwICAg&c=HAkYuh63rsuhr6Scbfh0UjBXeMK-ndb3voDTXcWzoCI&r=And7spKEXmRNIrq8pYCiSg&m=EHG6EmocNCDS6VoOZVxzZoCq-BqbJjBpsC9kXhGFTd0&s=_ChOm4nn2d9pTlDuA6I2EdYXc73LOiS_N4c0k0DOWPo&e= >> To unsubscribe, send any mail to >> "freebsd-arch-unsubscribe@freebsd.org" -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?79c3ce4d-0f4b-37f3-9e66-58a71d67962c>