-
Notifications
You must be signed in to change notification settings - Fork 847
Description
(This is moved from https://issues.apache.org/jira/browse/TS-1773).
Right now, we have a hotchpotch of various "string to integer" conversions happening in the core. We have a main one, ink_atoi() / ink_atoui64(). We should unify the code around this, such that we do not use atoi/strtol and other implementations.
@SolidWallOfCode Also suggests that we make these APIs a little more feature rich, including:
- Can take a pointer and length, not require null termination of input string
- Returns the number of characters parsed, or a pointer to the first unparsed character (like strtol)
- Works with 64 bit numbers
I think we already have most of this covered with the existing implementations in ParseRules.cc, with the exceptions of item 2 (end handles):
int64_t ink_atoi64(const char *);
uint64_t ink_atoui64(const char *);
int64_t ink_atoi64(const char *, int);
int ink_atoi(const char *str);
int ink_atoi(const char *str, int len);
unsigned int ink_atoui(const char *str);So, what I think is left doing here is (as the original Jira suggested):
- Go through the entire core code base, and find usage of string-to-int conversions (such as ::atoi, ::strtol, and possibly other implementations), and change them to use one of the above APIs as appropriate.
- Optional: @SolidWallOfCode Expresses a wish to refactor the implementations in ParseRules.cc / ParseRules.h, such that there is less code duplication (i.e. refactor with a single shared base function or some such).
- Optional: Expose two generic, public APIs to ts/ts.h.
These could/should be three different pull requests IMO. Also, it might make sense to move all of these APIs to a lib/ts/atoi{.cc,h} or some such, I don't think the name ParseRules.cc makes a lot sense here, but I'm ok either way.
Note that neither of these take an end handle, if we want that, we'd have to update the APIs accordingly of course (wouldn't be difficult). It might be necessary to support the public APIs below.
Item 3 is where we sort of got lost I think. But @SolidWallOfCode posted a suggested public API, that covers all potential use cases in a single, nice C-API (edited here for correctness on the naming):
int64_t TSatoi(const char* src, size_t n, const char** end, int radix);
uint64_t TSatou(const char* src, size_t n, const char** end, int radix);The implementation (in InkAPI.cc) for these two APIs would simply be wrappers over the existing ink_atoi() based APIs.