greiman
ValidDataLength Handling
The way you use the ValidDataLength is currently very wrong and can break when accessing files created on an ExFat system using linux's cp command to copy files. In linux when copying to an ExFat system and the file has trailing null bytes, the ValidDataLength of the file gets set to less then the DataLength.
I can easily test this by doing dd if=/dev/zero of=test/ bs=1M count=512 and then cping the resulting file to an exfat sdcard. This will create a file with the ValidDataLength set to 0x00 and the DataLength set to 0x20000000 and the NoFatChain bit is set indicating that the file is contiguous.
The problem is you seem to be using ValidDataLength to represent the size of the file when it is not, is always going to be the DataLength field. Currently when calling fileSize I get a value of 0 which is incorrect as the file should be 512MB in size and this is correctly reported on linux via ls -l or stat. In addition I have noticed other places where you incorrectly assume ValidDataLength is the size of the file. These include contiguousRange, which for the above file will return an endSector equal to beginSector since ValidFileSize is 0, seekSet returns an error if you try and seek beyond ValidDataLength, and read truncates the bytes requested to be read if they exceed ValidDataLength. There may be other issues but those are the obvious ones i found.
Reading the Microsoft Spec for the exFat system it seems clear that the ValidDataLength field should NOT be treated as the file size and instead is only used to determine what data in the file is uninitialized/undefined data and reads from within that range should return 0s.
7.6.5 ValidDataLength Field
The ValidDataLength field shall describe how far into the data stream user data has been written. Implementations shall update this field as they write data further out into the data stream. On the storage media, the data between the valid data length and the data length of the data stream is undefined. Implementations shall return zeroes for read operations beyond the valid data length.
If the corresponding File directory entry describes a directory, then the only valid value for this field is equal to the value of the DataLength field. Otherwise, the range of valid values for this field shall be:
At least 0, which means no user data has been written out to the data stream
At most DataLength, which means user data has been written out to the entire length of the data stream
Can you please update SdFat to be spec compliant in regards to ExFat? As it is now it fails to work prroperly when the files were written to the SdCard on linux.
I will follow this issue on BlueSCSI and then put any fix in SdFat.
Looks like contiguousRange() should be using m_dataLength instead of m_validLength.
The only way to determine the sectors allocated to a contiguous exFat file is to use the startSector and dataLength since the FAT is not used for contiguous files.
I also need to check all calculations with dataLength since dataLength could be anywhere in the final cluster.
It is amazing this works unless you pre-allocate clusters like the dd empty file. As long as validLength and dataLength are in the same cluster, calculations work using either value. This is what happens for most users that just write files.
I experimented with windows NT and it looks like the only way to have a file that behaves as expected in applications is to have ValidDataLength equal to DataLength.
Applications see zeros beyond ValidDataLength as the specified.
I need to find a description for the use of ValidDataLength.
I could implement read past ValidDataLength.
If you read or seek past ValidDataLength and then do a write what should happen?
Edit: looks like I would need to write zeros from ValidDataLength to the current position.
I implemented ExFat before Microsoft published the specification. I only had the data structures and a poor idea of how they were to be used.
Looks like there are fewer places to modify than I thought.
"m_dataLength" (13 hits in 3 files of 117 searched) "m_validLength" (17 hits in 4 files of 117 searched)
Yeah Microsoft did take an awfully long time to actually publish the official spec for ExFAT, so completely understandable that you didn't use the spec before it had been published.
The basic idea of ValidDataLength seems to be so you can implement some small optimizations if you know the data you would read for the disk is initialized zero data you can skip doing a disk read and just return a buffer full of zeros. This how linux is using from what i can tell, it sets the ValidDataLength to the place where all remaining data in the file are zeros. Though i think it only does so at cluster boundaries.
Yes most places you just need to update m_validData to reference m_dataLength, and there are not a whole lot of them. The two more involved spots require changing how read and writes work, with read returning zero filled buffers past the ValidDataLength and write as you said should write zeros to disk between ValidDataLength and the current position you started writing at.
The read part looks fairly easy. I just use the current read code to read up to validData then fill the user buffer with zeros until the position is dataLength.
Write is a bit more difficult but may not be too difficult since I already have code to fill directory clusters with zeros.
I finish mods for validDataLength problem. I did a few simple tests and and it passed but I want to do more tests.
seek() accepts any location in a file, read returns zeroes if you read beyond validDataLength, write fill the file with zeros before current position if it is after validDataLength.
I added uint64_t validLength() to FsFile so you can avoid reading past valid data for exFAT files. For FAT files it returns fileSize.
The mod took a smaller change than I expected, a diff shows fewer than 100 lines changed. The problem is that they are scattered through code I haven't looked at recently so I made them in the next version of SdFat-beta since an error could be subtle and cause major problems.
I will test them in a number of examples and write some new test. Soon I will post them to SdFat-beta. I can't risk putting them in the release version of SdFat.
Here are two test examples. I ran them on a Adafruit Metro RP2040 at 200 MHz.
The first example creates a 100 byte file with no validData then tests read beyond validData and then write with zero fill.
I ran this function on the file:
void goNogoTest() {
fillFirstSector(0xA5);
file.println("Test line.");
printFileStats("after first println");
uint8_t buf[20];
memset(buf, 0xFF, sizeof(buf));
int n = file.read(buf, sizeof(buf));
if ( n != sizeof(buf)) {
error("read size");
}
printFileStats("after read");
Serial.print("buf[0]: ");
Serial.println(buf[0]);
n = file.println("should be after zeros");
printFileStats("after second println");
Serial.print("n: ");
Serial.println(n); // should be 23
sd.dmpSector(&Serial, file.firstSector());
}
The output is what I expected, write moves validLength and fill in zeros if the file is positioned beyond validLength.
after create with open
firstCluster: 0
curCluster: 0
curPosition: 0
validLength: 0
dataLength: 0
isContiguous: false
after preAllocate
firstCluster: 8
curCluster: 0
curPosition: 0
validLength: 0
dataLength: 100
isContiguous: true
after first println
firstCluster: 8
curCluster: 8
curPosition: 12
validLength: 12
dataLength: 100
isContiguous: true
after read
firstCluster: 8
curCluster: 8
curPosition: 32
validLength: 12
dataLength: 100
isContiguous: true
buf[0]: 0
after second println
firstCluster: 8
curCluster: 8
curPosition: 55
validLength: 55
dataLength: 100
isContiguous: true
n: 23
0 54 65 73 74 20 6C 69 6E 65 2E 0D 0A 00 00 00 00
10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20 73 68 6F 75 6C 64 20 62 65 20 61 66 74 65 72 20
30 7A 65 72 6F 73 0D 0A A5 A5 A5 A5 A5 A5 A5 A5 A5
40 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5
50 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5
60 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5 A5
Here is how the file read on Windows with the PsPad editor in hex mode, the 0xA5 bytes after validLength read as zero.
Here is a test of how fast zero fill works for a large 5GB (GB = 1E9). I ran it on a Adafruit Metro RP2040 at 200 MHz.
void testWriteZeros() {
if (!file.seekEnd(-1)) {
error("seek write");
}
printFileStats("after seek for write");
uint32_t m = micros();
if (file.write((uint8_t)0xA5) != 1) {
error("write");
}
m = micros() - m;
printFileStats("after write");
Serial.print("micros: ");
Serial.println(m);
if (!file.seekEnd(-1)) {
error("seek read");
}
printFileStats("after seek for read");
Serial.print("read byte: 0x");
Serial.println(file.read(), HEX);
printFileStats("after read");
}
Here is the output. The 5GB zero fill took 263.6 seconds for a speed of 18,968,133 bytes per/sec.
after create with open firstCluster: 0 curCluster: 0 curPosition: 0 validLength: 0 dataLength: 0 isContiguous: false
after preAllocate firstCluster: 8 curCluster: 0 curPosition: 0 validLength: 0 dataLength: 5000000000 isContiguous: true
after seek for write firstCluster: 8 curCluster: 19081 curPosition: 4999999999 validLength: 0 dataLength: 5000000000 isContiguous: true
after write firstCluster: 8 curCluster: 19081 curPosition: 5000000000 validLength: 5000000000 dataLength: 5000000000 isContiguous: true micros: 263613218
after seek for read firstCluster: 8 curCluster: 19081 curPosition: 4999999999 validLength: 5000000000 dataLength: 5000000000 isContiguous: true read byte: 0xA5
after read firstCluster: 8 curCluster: 19081 curPosition: 5000000000 validLength: 5000000000 dataLength: 5000000000 isContiguous: true
I did find one additional bug with this example. I used size_t for the amount of zero fill but size_t is four bytes in the RP2040/RP2350 Arduino package.
I have finished several more tests and many example. I posted SdFat-beta Version 2.3.1-beta.2.
Is it possible for you try this beta?
Yeah I'll take a look at this over the weekend and try and get a BlueSCSI build against the SDFat-beta repro to see if it works.
I have been doing tests with very large file and I think I found a problem with files over 2^32 bytes. It could be a problem with the test.
You really need to get all casts correct for dealing with 64-bits. I had a case where seekEnd(int64_t offset) seem to fail.
seekEnd( -sizeof(Array)) failed. Then I found that seekEnd(-(int64_t)sizeof(Array)) works. In the first case seekEnd() gets a large positive argument.
I patterned seekEnd() after Linux lseek where for "SEEK_END - The file offset is set to the size of the file plus offset bytes".
Edit: I used Linux hexdump to prove the file is written correctly.
I found a problem in read() here. I will change the size_t to uint64_t and commit Version 2.3.1-beta.3.
Committed SdFat Version Version 2.3.1-beta.3. with the above change in exFAT read.
@brijohn could you try this build of 2.2.3 with your SD card and the image that was reporting the wrong size. I couldn't make a preallocated file on an exFat SD card to test with.
Just replace this line in the platformio.ini file with
SdFat=https://github.com/BlueSCSI/SdFat#3447b2eabb6c4d1e22c421b34d077bffecb7f81b
with this
SdFat=https://github.com/rabbitholecomputing/SdFat#2.2.3-gpt-exfat
Rebuild the target and reflash your board
Thanks for finding the bug!
And thank you Bill for the fix!
I got a build of BlueSCSI working with https://github.com/rabbitholecomputing/SdFat#2.2.3-gpt-exfat that includes exfat patches and i created two preallocated zero filled files on my sdcard.
One that was written contiguously and the other that i purposely fragmented. The contiguous one shows has the proper size detected and is correctly detected as contiguous, while the fragmented file properly warns that it is fragmented.
I was also able to correctly partition both images on MS-DOS using afdisk, and then install a few games onto them which appears to correctly work.
As it is it appears the fix that @greiman implemented seems to be properly handling the case where validDatalength is less then dataLength and using it with BlueSCSI seems to fix the issues i was having when adding images to the sdcard under linux.