Page 1 of 1

FAT sometimes returns invalid value

Posted: Mon Oct 12, 2009 11:55 am
by neon
Hello everyone,

Perhaps my math is wrong but something is wrong in my FAT parsing code in my boot libraries FAT12 code.

Whenever the contents on the disk gets larger, sometimes the value returned from the FAT (in NextCluster) is invalid. That is, it will contain a value that is greater then the maximum sectors on disk thus causing the system to crash when it tries to read it (or return 0 - file loaded is corrupt.)

Code: Select all

FsysFatRead (OUT OPT PBYTE Buffer,
         IN UINT Length) {

   BYTE FAT [SECTOR_SIZE];
   UINT FAT_Offset = 0, FAT_Sector = 0;
   USHORT NextCluster = 0, EntryOffset = 0;

//! *snip, just tests for valid parms, eof, locating the next sector & reading it

   //! find location of fat and next entry
   FAT_Offset = _FileInfo.CurrentCluster + ( _FileInfo.CurrentCluster / 2 ); // multiply by 1.5
   FAT_Sector = 1 + ( FAT_Offset / SECTOR_SIZE );
   EntryOffset = FAT_Offset % SECTOR_SIZE;

   //! read the FAT sector
   ArchDiskRawRead (_Drive, FAT_Sector, FAT);

   //! read entry for next cluster
   NextCluster = *( PUSHORT ) &FAT [ EntryOffset ];

   //! test if entry is odd or even
   if( _FileInfo.CurrentCluster & 0x0001 )
      NextCluster >>= 4;      //grab high 12 bits
   else
      NextCluster &= 0x0FFF;   //grab low 12 bits

   //! Sometimes reads that the next cluster is > maximum sector count?? :/
   BlpDisplayString ("\n\rFSysFatRead: Next Cluster=0x%x", NextCluster);

// *snip, test for FAT valid values

   _FileInfo.CurrentCluster = NextCluster;
}
This is starting to become more and more of an issue and has been going on for months. One thing that I have noticed was that, when the value returned was invalid, it was obtaining the value from FAT [511], the last byte of the sector.

Disk reading is done using the Bios int 0x13 function 2, thus there is no chance that a disk driver is at fault. It also works most of the time, but is increasingly starting to appear as the system grows larger thus becoming more and more of a problem.

Does anyone have any suggestions? Has anyone seen this before? I am happy to post more code if needed.

Thank you in advance :D

Re: FAT sometimes returns invalid value

Posted: Mon Oct 12, 2009 2:24 pm
by clange
neon wrote: Does anyone have any suggestions?
A 512 byte sector contains 341 and a third 12-bit values. You might be reading beyond the end of a sector. Your code doesn't seem to handle this corner case.

Hope it helps.

clange

Re: FAT sometimes returns invalid value

Posted: Mon Oct 12, 2009 6:31 pm
by pcmattman
Your code doesn't seem to handle this corner case.
To build on that, the spec itself mentions the case where a 12-bit entry crosses a sector boundary and even suggests reading sectors in pairs (page 17):
fatgen103.doc wrote:

Code: Select all

If(ThisFATEntOffset == (BPB_BytsPerSec – 1)) {
    /* This cluster access spans a sector boundary in the FAT      */
    /* There are a number of strategies to handling this. The      */
    /* easiest is to always load FAT sectors into memory           */
    /* in pairs if the volume is FAT12 (if you want to load        */
    /* FAT sector N, you also load FAT sector N+1 immediately      */
    /* following it in memory unless sector N is the last FAT      */
    /* sector). It is assumed that this is the strategy used here  */
    /* which makes this if test for a sector boundary span         */
    /* unnecessary.                                                */
}
My driver will always read in two sectors when working with FAT12's FAT (once the sectors are read they're cached for future use, before anyone brings up performance penalties) because it means I don't need to write extra logic for FAT12 - I've tried to write the driver so that it's as generic as possible across each FAT type.

Code: Select all

   //! Sometimes reads that the next cluster is > maximum sector count?? :/
   BlpDisplayString ("\n\rFSysFatRead: Next Cluster=0x%x", NextCluster);
I believe you were looking for "maximum cluster count," as clusters are not necessarily equal to sectors :)

Re: FAT sometimes returns invalid value

Posted: Mon Oct 12, 2009 6:56 pm
by neon
Thank you both for your responses :D

Im now reading two sectors for the FAT rather then one for a way around the problem. I am not sure if there are more problems related to the FAT12 code, however. I will post back if there are other issues that fall back on the filesystem code, or if problems persist.
I believe you were looking for "maximum cluster count," as clusters are not necessarily equal to sectors :)
Technically true, I meant sectors though because I was working with floppies at the time where clusters == sectors :) I suppose it was not the proper terminology used in FAT though.

Re: FAT sometimes returns invalid value

Posted: Thu Oct 15, 2009 5:17 am
by -m32
When dealing with FAT12, my driver just reads the entire FAT into memory. It's so small anyway :)

Re: FAT sometimes returns invalid value

Posted: Thu Oct 15, 2009 5:31 am
by pcmattman
When dealing with FAT12, my driver just reads the entire FAT into memory
I can't resist... Storytime! :)

In my FAT driver (12, 16, 32), I originally thought "Why not just read the entire FAT into memory and then write individual blocks back to disk when needed?" I thought it was the greatest idea in the world. It worked beautifully!

Then my FAT32 code simply would not work on real hardware, at all. It'd often cause crashes and out of memory errors.

When I realised I'd been reading the entire FAT into RAM, I felt a bit silly. Now it's managed by a proper block cache which is filled with blocks as they're read (after the first read they hit the cache instead of the actual disk).

NOTE: I'm not saying you're wrong. I just thought this would be a funny anecdote to your comment, because it was a silly decision of mine to think that a FAT32 FAT would be safe to read, in full, into memory.

Re: FAT sometimes returns invalid value

Posted: Fri Oct 16, 2009 4:44 am
by -m32
Yeah, reading an entire FAT32 FAT into memory is a completely different story. It can be a tad large :)