Hi, I have an M68k retro computing project I have been bringing back to life using FreeRTOS, FreeRTOS-Plus-CLI and FreeRTOS+FAT pretty successfully.
However, I seem to have discovered an issue when appending to an existing file using the FAT12 mode. This seems independent of the size of the original file or the amount of data being appended.
The issue can be easily repeatably created by creating a file of say 4kB then later (or immediately) appending repeating the pxFile = ff_fopen( pcSourceFile, “a” ); and ff_Write().
I can successfully write any size file as a single operation.
I suspect its an issue with seeking to the end of the cluster chain, though a cursory investigation appears that the file pointer is being moved correctly.
The subsequent append is always 2 clusters within the original file data and is again subsequently repeatable each time the append is performed.
I’m guessing that there’s not many users of the FAT12 mode of operation, but my original 1981 hardware supports 720k floppies which is convenient for my gcc development tool chain.
I’m happy to share my source code if you need but its very minimal to recreate the issue.
Cheers, Dave.
Hi @dsp-dave, thanks for your interest in FreeRTOS+FAT.
I’m guessing that there’s not many users of the FAT12 mode of operation
That is right. FAT12 has been implemented, but it hasn’t passed the strict tests from ff_stdio_tests_with_cwd.c, an old, well-known testing module, written by Richard Barry.
Also ff_format.c doesn’t have support for FAT12, only FAT16 and FAT32
Today, I created a FAT12 volume for testing in Ubuntu:
# Create an empty file
dd if=/dev/zero of=ramdisk.img bs=512 count=10240
# Initialise a FAT12 file system.
mkfs.vfat -F12 ramdisk.img
And than I ran some tests like you describe. But I COULD NOT replicate the problem.
Maybe it helps if you send me lengths of the files: what’s the size of the existing file, and how many bytes do you add?
Here is the function that I wrote for testing. I called it 4 times with these parameters:
ff_append /ram/ff_apped_01.txt 5263 3333
ff_append /ram/ff_apped_02.txt 8267 6666
ff_append /ram/ff_apped_03.txt 1071 9999
ff_append /ram/ff_apped_04.txt 511 15
ff_append.c (3.2 KB)
Thanks for your reply, the only difference I can see in your test is the first time you create the file with the “w” mode, whereas I’m always using the “a” mode even for the first creation of the file.
I’ll try your test code and see if I get a different result.
BTW I’m writing at least 4096 bytes (8 sectors) each time.
Dave.
I tested your ff_append code and it does indeed work correctly. The only difference to my implementations is that my write buffer size is 4096 bytes (> 1 sector) rather than your 256 byte write buffer (< 1 sector).
I have added a dynamic buffer size to your test and if the size is set > 1 sector size (512 bytes) then the test fails and the appended data starts at offset 0x0C00 rather than 0x1000 in the resulting file for my test call below.
Example call with 1k write buffer for 4k create and then 4k append:-
ff_append_test( “Tibosch.bin”,1024, 4096, 4096 );
I would attached the modified test code but I’m not allowed yet.
Cheers, Dave.
You should be able to attach now.
OK Here You Go…
ff_append.c (3.5 KB)
The test runs well, I think that the result files show what we expect.
You used free(pcBuffer) in stead of vPortFree(pcBuffer), and that made my application crash. Maybe for you they are equivalent.
I zipped the two resulting files:
ff_append_result.zip (540 Bytes)
ff_apped_05b.txt = the 4 KB file before the append
ff_apped_05.txt = the 8 KB file after the append
The file was appended by opening it with with “a”, so calling ff_fseek wasn’t necessary.
The following commands should do the same:
pxHandle = ff_fopen( pcFilename, "+" ); // update mode
ff_fseek( pxHandle, 0, FF_SEEK_END );
Opening with “a” will seek implicitly:
pxHandle = ff_fopen( pcFilename, "a" ); // append mode
Maybe you can show the resulting files of you test?
Your testing code outputs the zero character, which makes it more difficult to compare the files. I made the following changes:
if( ( uxBufferSize - uxBufferFilled ) < uxLineLength )
{
- uxLineLength = uxBufferSize - uxBufferFilled)-1;
+ uxLineLength = uxBufferSize - uxBufferFilled;
}
lu16lineCount++;
} while( uxBufferFilled < ( uxBufferSize - 1 ) );
- pcCursor[ uxBufferSize - 1 ] = '\0';
Let the last bytes be a CR/LF:
size_t uxCount = uxBufferSize;
- if( uxCount > uxLeft )
+ if( uxCount >= uxLeft )
{
uxCount = uxLeft;
/* This is the last write. */
if( uxCount >= 2 )
{
pcBuffer[ uxCount - 2 ] = 13;
pcBuffer[ uxCount - 1 ] = 10;
}
}
int rc = ff_fwrite( pcBuffer, 1, uxCount, pxHandle );
Now I did the following tests:
ff_append /ram/ff_apped_01.txt 5263 3333
ff_append /ram/ff_apped_02.txt 8267 6666
ff_append /ram/ff_apped_03.txt 1071 9999
ff_append /ram/ff_apped_04.txt 6144 3074
Here is s screenshot from winmerge:
Hi Htbosch.
Updated the test as per your patch, output is the same regardless of UPDATE_MODE in use.
My implementation is based on the 160908 release which is not the latest which seems to have a lot of refactoring. I have attached my result files for you to see.
Also an 8kB file created in one go using:-
ff_append_test( “Tibosh.txt”,1024, 8192, 0 );
AppendTestResults.zip (935 Bytes)
I’ll look at incorporating the latest version which is I guess what you’ve been using to do your testing and creating some new sector server unit tests.
Cheers.
Dave
Hi Htbosch,
To update you with my progress.
I added some sector server diagnostics and it could be seen that the append write requests were indeed within the last 2 sectors of the previous data.
So I ported the latest download release (can’t seem to see a version number anymore?).
Re running the append test the sector write requests are now correctly after the first data written and all is working correctly.
There has been quite a lot of rewriting/refactoring compared to the previous version I started with but this issue seems to have been fixed.
The only issue I can see now is that the pxIOManager->xPartition.pcVolumeLabel isn’t being correctly set as the media has a label but the string is set to the default ‘NO NAME’ string. I think this may be due to being formatted in Windows.
Thanks for all your assistance.
Cheers, Dave.
Thank you for reporting back!
Hi @dsp-dave , thank you for reporting back.
I was indeed testing with the latest release of FreeRTOS+FAT, and I couldn’t replicate the problem.
You wrote:
pcVolumeLabel isn’t being correctly set as the media has a label but the string is set to the default ‘NO NAME’ string
I will check that too. Doesn’t sound too difficult.