CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to support@ccsinfo.com

bug in FAT.c library

 
Post new topic   Reply to topic    CCS Forum Index -> Code Library
View previous topic :: View next topic  
Author Message
martind1983



Joined: 22 Mar 2013
Posts: 16

View user's profile Send private message

bug in FAT.c library
PostPosted: Sun Mar 31, 2013 5:57 am     Reply with quote

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

View user's profile Send private message

PostPosted: Sun Mar 31, 2013 4:34 pm     Reply with quote

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

View user's profile Send private message

PostPosted: Mon Apr 01, 2013 11:55 am     Reply with quote

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

View user's profile Send private message

PostPosted: Mon Apr 01, 2013 12:34 pm     Reply with quote

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.
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> Code Library All times are GMT - 6 Hours
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group