| View previous topic :: View next topic |
| Author |
Message |
martind1983
Joined: 22 Mar 2013
Posts: 16
|
| bug in FAT.c library |
 Posted: Sun Mar 31, 2013 5:57 am |
 |
|
Hi.
I have feeling, that I found bug in FAT.c library.
When I used fatopen() function with FAST_FAT directive active
There is this sequence of original code:
// start looking for the file, start at root
cur_stream.Start_Addr = cur_stream.Parent_Start_Addr = Root_Dir;
while(fname[fname_parse_pos] != '\0')
{
target_file[fname_parse_pos - 1] = fname[fname_parse_pos];
fname_parse_pos += 1;
}
target_file[fname_parse_pos] = '\0';
which looks for filename (fname[] from parameter of function) and copy all characters except directory character and NULL terminator to temporary string (target_file) in while loop. After it adds NULL terminator
at the end of new string "red colored". And there is a bug "fname_parse_pos" has to be "fname_parse_pos - 1" otherwise it also adds to the string garbage from memory and then NULL terminator and by this solution it creates new temporary file name which is different from original and it is not able to find it and function returns EOF
There is this sequence of corrected code:
// start looking for the file, start at root
cur_stream.Start_Addr = cur_stream.Parent_Start_Addr = Root_Dir;
while(fname[fname_parse_pos] != '\0')
{
target_file[fname_parse_pos - 1] = fname[fname_parse_pos];
fname_parse_pos += 1;
}
target_file[fname_parse_pos - 1] = '\0'; |
|
 |
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Sun Mar 31, 2013 4:34 pm |
|
|
Nice find!
You didn't say which version of the compiler you used, but the bug is still present in my v4.141
Note the following from the fat.c source file comments: | Quote: | // NOTE For faster writing or appending for an application such as a logger,
// uncomment #FAST_FAT below. This will make the FAT library assume
// there is one file on the card to write or append to, thereby
// making writing and appending much faster. Reading is impossible in
// this mode.
// THIS IS NOT TESTED VERY WELL YET! |
Seems like CCS is aware of poor quality.
Just a few more remarks on that little part:
- Copying a string using a for loop is much less efficient than the strcpy() function, more difficult to read and error prone (as we have seen).
- The whole copy function can be left out, it is only stripping the first character from the string. It suffices to use the original string with a 1 position offset.
Saving one integer and an array, the code could then become: | Code: | signed int fatopen(char fname[], char mode[], FILE* stream)
{
FILE cur_stream; // this will be the stream that will be returned if all goes well
#ifndef FAST_FAT
int fname_parse_pos = 1; // the current index of the fname character
char target_file[MAX_FILE_NAME_LENGTH]; // temporary buffer to hold names of files
int
depth = 0, // how many subdirectories deep the file is
target_file_parse_pos; // the current index of the target_file character
#endif // #ifndef FAST_FAT
// set flags
#ifdef FAST_FAT
switch(mode[0])
{
case 'w':
cur_stream.Flags = Write;
break;
case 'a':
cur_stream.Flags = Append;
break;
default:
return EOF;
}
// start looking for the file, start at root
cur_stream.Start_Addr = cur_stream.Parent_Start_Addr = Root_Dir;
// find the file inside of its subdirectory
// Start at fname[1] to skip the '/' character
if(set_file(&fname[1], 0x20, &cur_stream) != GOODEC)
{
cur_stream.Flags |= File_Not_Found;
*stream = cur_stream;
return EOF;
}
// at this point, we've found the file
*stream = cur_stream;
return GOODEC;
#else // NO FAST_FAT |
I've sent a bug report to CCS. |
|
|
martind1983
Joined: 22 Mar 2013
Posts: 16
|
|
| Posted: Mon Apr 01, 2013 11:55 am |
|
|
| Did you already try to pass that sequence of code you put it here. I have feeling you can't use fname[1] in set_file function because the parameter has to be base address of array it's used as pointer to array and not first value. But I don't know if my statement is correct. That is just old ANSI C rule. |
|
|
martind1983
Joined: 22 Mar 2013
Posts: 16
|
|
| Posted: Mon Apr 01, 2013 12:34 pm |
|
|
| martind1983 wrote: | | Did you already try to pass that sequence of code you put it here. I have feeling you can't use fname[1] in set_file function because the parameter has to be base address of array it's used as pointer to array and not first value. But I don't know if my statement is correct. That is just old ANSI C rule. |
I already found it your code should be correct. I was not aware that fact. I didn't use it long time so you know I forgot. Sorry.
Martin. |
|
|
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
