CP-50259 simplify parsing size with kib, mib, etc suffix #5819
Conversation
ocaml/xapi-cli-server/record_util.ml
Outdated
| record_failure | ||
| "Failed to parse field '%s': expecting an integer (possibly with suffix \ | ||
| KiB, MiB, GiB, TiB), got '%s'" | ||
| field x | ||
|
|
||
| (* Vincent's random mac utils *) |
There was a problem hiding this comment.
I'm not sure this Vincent is here anymore, do we want to keep this?
There was a problem hiding this comment.
Don't have to. I was not not sure what this is about.
There was a problem hiding this comment.
He's sitting right next to me, though 😅
There was a problem hiding this comment.
Definitely a different Vincent ;)
| let alldigit = ref true and i = ref (String.length s - 1) in | ||
| while !alldigit && !i > 0 do | ||
| alldigit := Astring.Char.Ascii.is_digit s.[!i] ; | ||
| decr i | ||
| done ; | ||
| if !alldigit then | ||
| record_failure | ||
| "Failed to parse field '%s': number too big (maximum = %Ld TiB)" field | ||
| max_size_TiB |
There was a problem hiding this comment.
Is it not important anymore to distinguish between "not an integer - couldn't parse" and "integer - but too big"? Additionally, we could overflow above when multiplying ** 1024 several times, do we want to catch and report that?
There was a problem hiding this comment.
Overflow is detected and results in an exception, which is caught. That number is incredibly large, though.
Simplify the implementation. Signed-off-by: Christian Lindig <christian.lindig@cloud.com>
| GiB or TiB)" | ||
| field x | ||
| in | ||
| (* FIXME: detect overflow *) |
There was a problem hiding this comment.
see comment above re: overflow
lindig
force-pushed
the
private/christianlin/CP-50259
branch
from
c79a3fc
to
e1fd10b
Compare
|
Didn't the old function accept a space between the number and suffix, while the new one does not? |
|
Reading the code, I believe you are right that the splitting allows a space. It's a bit awkward though, because this gets in the way of shell splitting words - so you would have quote that in a shell. Let me see if this can be accommodated. |
Simplify the implementation.