Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using size_t limits max number to 2^32. #739

Merged
merged 7 commits into from
May 22, 2024

Conversation

FreeBear-nc
Copy link
Contributor

When dealing with SD cards we could be asked to print a uint64 (for example, from SD.totalGytes()).
This attempt to handle the very large numbers that might be expected from an SD card.

Yes, there is a bit of ugliness from recasting to a uint32 before calling the snprintf function, but this avoids implicit casts where data could be truncated.

we could be asked to print a uint64 (for example, from SD.totalGytes()).
@marsman7
Copy link
Contributor

marsman7 commented May 22, 2024

filesize = filesize / (D_FILE_SIZE_DIVIDER/100);
With this code you loose resolution. In Example

original

#define D_FILE_SIZE_DIVIDER 1024

filesize = 16.777.216;
filesize *= 100;  // 1.677.721.600
filesize = filesize / D_FILE_SIZE_DIVIDER;  // 1.638.400

your code

#define D_FILE_SIZE_DIVIDER 1024

filesize = 16.777.216;
filesize = filesize / (D_FILE_SIZE_DIVIDER/100);  // 1.677.721

@FreeBear-nc
Copy link
Contributor Author

Noted.
Was trying to avoid an overflow when multiplying by 100 which the current code will do at (approx) 2^28. But with a 2^64 limit, it is going to be a very, very long time before we see file sizes in the Exbi range (that is 2^60).

@fvanroie
Copy link
Collaborator

With uint64_t you can still do the *100 before dividing by D_FILE_SIZE_DIVIDER and keep the resolution.
It will indeed only pose a problem with unintelligible numbers.

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).
@FreeBear-nc
Copy link
Contributor Author

Reworked the code to use an array of suffixes and reduce the number of snprintf_P statements. This means if TiB or others are needed, they just need to be added to the array.

@fvanroie fvanroie merged commit 07ecdcf into HASwitchPlate:master May 22, 2024
@fvanroie
Copy link
Collaborator

Thanks, I did a bit more refactoring to make the code even more universal, and added Terabytes. cc1d6d9
It should also sidestep the overflow issue entirely now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants