From aa09f4bfd43a396816d902428a12a2b940d2f486 Mon Sep 17 00:00:00 2001 From: FreeBear Date: Wed, 22 May 2024 00:51:02 +0100 Subject: [PATCH 1/7] Using size_t limits max number to 2^32. When dealing with SD cards we could be asked to print a uint64 (for example, from SD.totalGytes()). --- src/hasp/hasp_parser.cpp | 26 ++++++++++++++------------ src/hasp/hasp_parser.h | 2 +- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/hasp/hasp_parser.cpp b/src/hasp/hasp_parser.cpp index eee1175d..f6db3854 100644 --- a/src/hasp/hasp_parser.cpp +++ b/src/hasp/hasp_parser.cpp @@ -197,24 +197,26 @@ bool Parser::is_only_digits(const char* s) return strlen(s) == digits; } -int Parser::format_bytes(size_t filesize, char* buf, size_t len) +int Parser::format_bytes(uint64_t filesize, char* buf, size_t len) { - if(filesize < D_FILE_SIZE_DIVIDER) return snprintf_P(buf, len, PSTR("%d " D_FILE_SIZE_BYTES), filesize); - filesize = filesize * 100; + uint32_t tmp = (uint32_t) filesize; // cast to unsigned int here to saye ugly casts in next line + if(filesize < D_FILE_SIZE_DIVIDER) return snprintf_P(buf, len, PSTR("%u " D_FILE_SIZE_BYTES), tmp); - filesize = filesize / D_FILE_SIZE_DIVIDER; // multiply by 100 for 2 decimal place + filesize = filesize / (D_FILE_SIZE_DIVIDER/100); // multiply by 100 for 2 decimal place + tmp = (uint32_t) filesize; if(filesize < D_FILE_SIZE_DIVIDER * 100) - return snprintf_P(buf, len, PSTR("%d" D_DECIMAL_POINT "%02d " D_FILE_SIZE_KILOBYTES), filesize / 100, - filesize % 100); + return snprintf_P(buf, len, PSTR("%u" D_DECIMAL_POINT "%02u " D_FILE_SIZE_KILOBYTES), tmp / 100, + tmp % 100); - filesize = filesize / D_FILE_SIZE_DIVIDER; // multiply by 100 for 2 decimal place + filesize = filesize / D_FILE_SIZE_DIVIDER; + tmp = (uint32_t) filesize; if(filesize < D_FILE_SIZE_DIVIDER * 100) - return snprintf_P(buf, len, PSTR("%d" D_DECIMAL_POINT "%02d " D_FILE_SIZE_MEGABYTES), filesize / 100, - filesize % 100); + return snprintf_P(buf, len, PSTR("%u" D_DECIMAL_POINT "%02u " D_FILE_SIZE_MEGABYTES), tmp / 100, + tmp % 100); - filesize = filesize / D_FILE_SIZE_DIVIDER; // multiply by 100 for 2 decimal place - return snprintf_P(buf, len, PSTR("%d" D_DECIMAL_POINT "%02d " D_FILE_SIZE_GIGABYTES), filesize / 100, - filesize % 100); + tmp = (uint32_t) (filesize / D_FILE_SIZE_DIVIDER); + return snprintf_P(buf, len, PSTR("%u" D_DECIMAL_POINT "%02u " D_FILE_SIZE_GIGABYTES), tmp / 100, + tmp % 100); } uint8_t Parser::get_action_id(const char* action) diff --git a/src/hasp/hasp_parser.h b/src/hasp/hasp_parser.h index d1c44bea..84dcfd1b 100644 --- a/src/hasp/hasp_parser.h +++ b/src/hasp/hasp_parser.h @@ -19,7 +19,7 @@ class Parser { static bool is_true(const char* s); static bool is_true(JsonVariant json); static bool is_only_digits(const char* s); - static int format_bytes(size_t filesize, char* buf, size_t len); + static int format_bytes(uint64_t filesize, char* buf, size_t len); }; #ifndef ARDUINO From 33f3c3691576a187057aa98039d8812e77cdfe0b Mon Sep 17 00:00:00 2001 From: FreeBear Date: Wed, 22 May 2024 11:20:14 +0100 Subject: [PATCH 2/7] Take onboard comment about loss of precision. Using size_t would have overflowed at (approx) 2^28. But with a uint64_t the limit is going to be up in the Exbi range (2^60). --- src/hasp/hasp_parser.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/hasp/hasp_parser.cpp b/src/hasp/hasp_parser.cpp index f6db3854..2f79ffb6 100644 --- a/src/hasp/hasp_parser.cpp +++ b/src/hasp/hasp_parser.cpp @@ -199,22 +199,24 @@ bool Parser::is_only_digits(const char* s) int Parser::format_bytes(uint64_t filesize, char* buf, size_t len) { + filesize *= 100; // Warning - If filesize up in the Zi range (2^60), we will overflow. uint32_t tmp = (uint32_t) filesize; // cast to unsigned int here to saye ugly casts in next line if(filesize < D_FILE_SIZE_DIVIDER) return snprintf_P(buf, len, PSTR("%u " D_FILE_SIZE_BYTES), tmp); - filesize = filesize / (D_FILE_SIZE_DIVIDER/100); // multiply by 100 for 2 decimal place + filesize /= D_FILE_SIZE_DIVIDER; tmp = (uint32_t) filesize; if(filesize < D_FILE_SIZE_DIVIDER * 100) return snprintf_P(buf, len, PSTR("%u" D_DECIMAL_POINT "%02u " D_FILE_SIZE_KILOBYTES), tmp / 100, tmp % 100); - filesize = filesize / D_FILE_SIZE_DIVIDER; + filesize /= D_FILE_SIZE_DIVIDER; tmp = (uint32_t) filesize; - if(filesize < D_FILE_SIZE_DIVIDER * 100) + if(filesize < D_FILE_SIZE_DIVIDER * D_FILE_SIZE_DIVIDER * 100) return snprintf_P(buf, len, PSTR("%u" D_DECIMAL_POINT "%02u " D_FILE_SIZE_MEGABYTES), tmp / 100, tmp % 100); - tmp = (uint32_t) (filesize / D_FILE_SIZE_DIVIDER); + filesize /= D_FILE_SIZE_DIVIDER; + tmp = (uint32_t) filesize; return snprintf_P(buf, len, PSTR("%u" D_DECIMAL_POINT "%02u " D_FILE_SIZE_GIGABYTES), tmp / 100, tmp % 100); } From 7dc3de34f82454b255df143facd0c3ce67f1ad7a Mon Sep 17 00:00:00 2001 From: FreeBear Date: Wed, 22 May 2024 11:56:23 +0100 Subject: [PATCH 3/7] One too many file system dividers used for megabyte comparison. --- src/hasp/hasp_parser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hasp/hasp_parser.cpp b/src/hasp/hasp_parser.cpp index 2f79ffb6..64e4729f 100644 --- a/src/hasp/hasp_parser.cpp +++ b/src/hasp/hasp_parser.cpp @@ -211,7 +211,7 @@ int Parser::format_bytes(uint64_t filesize, char* buf, size_t len) filesize /= D_FILE_SIZE_DIVIDER; tmp = (uint32_t) filesize; - if(filesize < D_FILE_SIZE_DIVIDER * D_FILE_SIZE_DIVIDER * 100) + if(filesize < D_FILE_SIZE_DIVIDER * 100) return snprintf_P(buf, len, PSTR("%u" D_DECIMAL_POINT "%02u " D_FILE_SIZE_MEGABYTES), tmp / 100, tmp % 100); From b65bd46b411b44c44a8b51b3310b942a66f7e118 Mon Sep 17 00:00:00 2001 From: FreeBear Date: Wed, 22 May 2024 12:18:03 +0100 Subject: [PATCH 4/7] 2^60 is Exbi (Ei) not Zebi (Zi) --- src/hasp/hasp_parser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hasp/hasp_parser.cpp b/src/hasp/hasp_parser.cpp index 64e4729f..fb557cc5 100644 --- a/src/hasp/hasp_parser.cpp +++ b/src/hasp/hasp_parser.cpp @@ -199,7 +199,7 @@ bool Parser::is_only_digits(const char* s) int Parser::format_bytes(uint64_t filesize, char* buf, size_t len) { - filesize *= 100; // Warning - If filesize up in the Zi range (2^60), we will overflow. + filesize *= 100; // Warning - If filesize up in the Ei range (2^60), we will overflow. uint32_t tmp = (uint32_t) filesize; // cast to unsigned int here to saye ugly casts in next line if(filesize < D_FILE_SIZE_DIVIDER) return snprintf_P(buf, len, PSTR("%u " D_FILE_SIZE_BYTES), tmp); From 452c619e97c66698112641503d4a1bb419461e57 Mon Sep 17 00:00:00 2001 From: FreeBear Date: Wed, 22 May 2024 12:44:36 +0100 Subject: [PATCH 5/7] Use a const char* array for format_bytes suffixes at the expense of a little more system memory. --- src/hasp/hasp_parser.cpp | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/hasp/hasp_parser.cpp b/src/hasp/hasp_parser.cpp index fb557cc5..ae5fe88c 100644 --- a/src/hasp/hasp_parser.cpp +++ b/src/hasp/hasp_parser.cpp @@ -203,22 +203,14 @@ int Parser::format_bytes(uint64_t filesize, char* buf, size_t len) uint32_t tmp = (uint32_t) filesize; // cast to unsigned int here to saye ugly casts in next line if(filesize < D_FILE_SIZE_DIVIDER) return snprintf_P(buf, len, PSTR("%u " D_FILE_SIZE_BYTES), tmp); - filesize /= D_FILE_SIZE_DIVIDER; + const char* suffix[] = {D_FILE_SIZE_KILOBYTES, D_FILE_SIZE_MEGABYTES, D_FILE_SIZE_GIGABYTES}; + int n = -1; + while (filesize > D_FILE_SIZE_DIVIDER * 100) { + n += 1; + filesize /= D_FILE_SIZE_DIVIDER; + } tmp = (uint32_t) filesize; - if(filesize < D_FILE_SIZE_DIVIDER * 100) - return snprintf_P(buf, len, PSTR("%u" D_DECIMAL_POINT "%02u " D_FILE_SIZE_KILOBYTES), tmp / 100, - tmp % 100); - - filesize /= D_FILE_SIZE_DIVIDER; - tmp = (uint32_t) filesize; - if(filesize < D_FILE_SIZE_DIVIDER * 100) - return snprintf_P(buf, len, PSTR("%u" D_DECIMAL_POINT "%02u " D_FILE_SIZE_MEGABYTES), tmp / 100, - tmp % 100); - - filesize /= D_FILE_SIZE_DIVIDER; - tmp = (uint32_t) filesize; - return snprintf_P(buf, len, PSTR("%u" D_DECIMAL_POINT "%02u " D_FILE_SIZE_GIGABYTES), tmp / 100, - tmp % 100); + return snprintf_P(buf, len, PSTR("%u" D_DECIMAL_POINT "%02u %s"), tmp / 100, tmp % 100, suffix[n]); } uint8_t Parser::get_action_id(const char* action) From e37ca4c318e41e767f6d76fff3e2277700146906 Mon Sep 17 00:00:00 2001 From: FreeBear Date: Wed, 22 May 2024 15:21:44 +0100 Subject: [PATCH 6/7] Just in case file size in the TiB range gets passed, make sure we don't index beyond the end of the array. --- src/hasp/hasp_parser.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/hasp/hasp_parser.cpp b/src/hasp/hasp_parser.cpp index ae5fe88c..b3966380 100644 --- a/src/hasp/hasp_parser.cpp +++ b/src/hasp/hasp_parser.cpp @@ -199,16 +199,18 @@ bool Parser::is_only_digits(const char* s) int Parser::format_bytes(uint64_t filesize, char* buf, size_t len) { - filesize *= 100; // Warning - If filesize up in the Ei range (2^60), we will overflow. + filesize *= 102400; // Warning - If filesize up in the Ei range (2^60), we will overflow. uint32_t tmp = (uint32_t) filesize; // cast to unsigned int here to saye ugly casts in next line if(filesize < D_FILE_SIZE_DIVIDER) return snprintf_P(buf, len, PSTR("%u " D_FILE_SIZE_BYTES), tmp); - const char* suffix[] = {D_FILE_SIZE_KILOBYTES, D_FILE_SIZE_MEGABYTES, D_FILE_SIZE_GIGABYTES}; + const char* suffix[] = {D_FILE_SIZE_KILOBYTES, D_FILE_SIZE_MEGABYTES, D_FILE_SIZE_GIGABYTES, "oops"}; + #define UNKNOWN_SUFFIX 2 int n = -1; - while (filesize > D_FILE_SIZE_DIVIDER * 100) { + while (filesize > D_FILE_SIZE_DIVIDER * 100 && n < UNKNOWN_SUFFIX) { n += 1; filesize /= D_FILE_SIZE_DIVIDER; } + tmp = (uint32_t) filesize; return snprintf_P(buf, len, PSTR("%u" D_DECIMAL_POINT "%02u %s"), tmp / 100, tmp % 100, suffix[n]); } From fcd99682b32358be20f8749741f87cb27f30904a Mon Sep 17 00:00:00 2001 From: FreeBear Date: Wed, 22 May 2024 15:26:31 +0100 Subject: [PATCH 7/7] Forgot to roll back on a test. --- src/hasp/hasp_parser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hasp/hasp_parser.cpp b/src/hasp/hasp_parser.cpp index b3966380..6177164d 100644 --- a/src/hasp/hasp_parser.cpp +++ b/src/hasp/hasp_parser.cpp @@ -199,7 +199,7 @@ bool Parser::is_only_digits(const char* s) int Parser::format_bytes(uint64_t filesize, char* buf, size_t len) { - filesize *= 102400; // Warning - If filesize up in the Ei range (2^60), we will overflow. + filesize *= 100; // Warning - If filesize up in the Ei range (2^60), we will overflow. uint32_t tmp = (uint32_t) filesize; // cast to unsigned int here to saye ugly casts in next line if(filesize < D_FILE_SIZE_DIVIDER) return snprintf_P(buf, len, PSTR("%u " D_FILE_SIZE_BYTES), tmp);