Fix SD info64 : info.usedBytes calculation giving weird result#8445
Fix SD info64 : info.usedBytes calculation giving weird result#8445earlephilhower merged 1 commit intoesp8266:masterfrom
Conversation
| info.maxPathLength = 255; // TODO ? | ||
| info.totalBytes =_fs.vol()->clusterCount() * info.blockSize; | ||
| info.usedBytes = info.totalBytes - (_fs.vol()->freeClusterCount() * _fs.vol()->bytesPerCluster()); | ||
| info.usedBytes = (_fs.vol()->clusterCount() - _fs.vol()->freeClusterCount()) * info.blockSize; |
There was a problem hiding this comment.
When I manually factor things I think this change is mathematically the same as the existing code.
info.usedBytes = info.totalBytes - (_fs.vol()->freeClusterCount() * _fs.vol()->bytesPerCluster());
info.totalBytes =_fs.vol()->clusterCount() * info.blockSize;
info.blockSize = _fs.vol()->bytesPerCluster();
So,
info.usedBytes = (_fs.vol()->clusterCount() * _fs.vol()->bytesPerCluster()) - (_fs.vol()->freeClusterCount() * _fs.vol()->bytesPerCluster());
Factor out the bpc and you get
info.usedBytes = (_fs.vol()->clusterCount() - _fs.vol()->freeClusterCount() ) * _fs.vol()->bytesPerCluster();
which is the patch's rewrite, no?
There was a problem hiding this comment.
I do agree about calculation , but the fact is the current does not give the correct result.
Seems overflow happen.
Could be a type issue when doing calculation ?
There was a problem hiding this comment.
with figures it is may be more clear:
#include <SPI.h>
#include <SD.h>
#include <SDFS.h>
void setup(){
Serial.begin(115200);
if(!SD.begin(5)){
Serial.println("Card Mount Failed");
return;
}
FSInfo64 info;
if (!SDFS.info64(info)) {
Serial.println("Size unkown");
return ;
}
Serial.printf("\nTotal space: %lluMB\n", info.totalBytes / (1024 * 1024));
Serial.printf("Used space: %lluMB\n", info.usedBytes / (1024 * 1024));
}
void loop(){}
Without PR
Total space: 3760MB
Used space: 4929MB =>Wrong
With PR
Total space: 3760MB
Used space: 833MB =>Correct
earlephilhower
left a comment
earlephilhower
left a comment
There was a problem hiding this comment.
Thanks for giving the example. You're probably right, there is some type promotion issue and 32b overflow in the existing code.
|
Maybe we would want explicit casts to u64? There are mixed i32, u16 and u32, while we expect the result to be u64-wide and not u32 cast into one in the end |
3 participants
No description provided.