pitrou commented on a change in pull request #11064:
URL: https://github.com/apache/arrow/pull/11064#discussion_r704585027
##########
File path: cpp/src/arrow/util/value_parsing.h
##########
@@ -273,6 +273,36 @@ inline bool ParseUnsigned(const char* s, size_t length,
uint64_t* out) {
#undef PARSE_UNSIGNED_ITERATION
#undef PARSE_UNSIGNED_ITERATION_LAST
+
+
+template <typename T>
+bool ParseHex(const char* s, size_t length, T* out) {
+ T result = 0;
+ int num_iterations = (int)(sizeof(T)*2);
Review comment:
Since we already know that `length` will be `<= sizeof(T)*2`, it does
not seem useful to have this. Just loop on `length`.
##########
File path: cpp/src/arrow/util/value_parsing.h
##########
@@ -281,6 +311,20 @@ struct StringToUnsignedIntConverterMixin {
if (ARROW_PREDICT_FALSE(length == 0)) {
return false;
}
+ // If its starts with 0x then its hex
Review comment:
"it", "it's"
##########
File path: cpp/src/arrow/util/value_parsing.h
##########
@@ -281,6 +311,20 @@ struct StringToUnsignedIntConverterMixin {
if (ARROW_PREDICT_FALSE(length == 0)) {
return false;
}
+ // If its starts with 0x then its hex
+ if (*s == '0' && ((*(s + 1) == 'x') || (*(s + 1) == 'X'))){
Review comment:
You can't access `*(s + 1)` before checking that `length` is large
enough. Also, since `0x` is forbidden, might immediately check that `length >=
3`.
(also, nit: `s[1]` is clearer than `*(s + 1)`)
##########
File path: cpp/src/arrow/util/value_parsing.h
##########
@@ -329,13 +373,31 @@ struct StringToSignedIntConverterMixin {
if (ARROW_PREDICT_FALSE(length == 0)) {
return false;
}
+
+ // If its starts with 0x then its hex
+ if (*s == '0' && ((*(s + 1) == 'x') || (*(s + 1) == 'X'))){
+ length -= 2;
+ s += 2;
+
+ // lets make sure that the length of the string is not too big
+ if (!ARROW_PREDICT_TRUE(sizeof(unsigned_value)*2 >= length && length >
0)) {
Review comment:
This could be part of `ParseHex` instead of being repeated twice.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
