FAT12 file problems

[accelleon]

Sorry about that I didn't even check to see if it was fully uploaded.
I just uploaded to my site's server:

Download FAT.cpp

Take a look at that.

[neon]

Why are you using uninitialized pointers everywhere?
*and pointers set to 0 are being used?

[accelleon]

If your talking about the ones in the structs Visual C++ doesn't let me initialize variables that aren't integral or aren't a static constant. Anything else that's uninitialized is probably because one time when I tried to initialize a variable (forget which one) it triple faulted. Driver = .

I will try to initialize all of the other variables but if it triple faults...

[neon]

No, Im referring to your code like this:

Code: Select all

int findfile (char *fname)
{
	unsigned short entry = 1;
	char *testfname;
	DebugPrintf("\nLooking for file: %s\n\n", fname);
	while (true)
	{
		for(int i=0; i<11; i++)
			testfname[i] = RootDir_MemLocation[(32 * (entry-1)) + i]; // where does testfname point?

The above occurs everywhere throughout your code. Along with code like:

Code: Select all

void* _cdecl fread (unsigned char *fp, unsigned int offset, unsigned int count, bool readasint)
{
	unsigned char *buff = 0;
	for(int i=0; i<count; i++)
	{
		buff[i] = fp[offset + count]; // if not in virtual memory, say good bye to the IVT

Do you see the problem in the above code? (Comments added by me.)

[accelleon]

Thanks, I got my findfile function to work, the strcmp function you had in your standard lib didn't work for that function, I replaced it with my own and it worked. I still have issues though after I got findfile to work I put in one DebugPrintf and found out that for some odd reason the file size section of the directory entry was missing. I'm very confused as of now:
In Microsoft's specs they say the last 4 bytes of the directory entry are the file size.
But in the majority of other specs they say its reserved.

I was reading the file size to try to find a way to read entire files without using the FAT to follow clusters (I'll admit: sort of a lazy replacement). But anyway anyone got an example of how to follow clusters using the FAT; The loading part should be fine.

Code: Select all

void* _cdecl fread (unsigned char *fp, unsigned int offset, unsigned int count, bool readasint)
{
	unsigned char *buff = 0;
	for(int i=0; i<count; i++)
	{
		buff[i] = fp[offset + count]; // if not in virtual memory, say good bye to the IVT

If you thought that then you'd be surprised to know that that code actually worked fine . (for once in my FAT12 driver )

[pcmattman]

Hi,

I was reading the file size to try to find a way to read entire files without using the FAT to follow clusters... <snip>... In Microsoft's specs they say the last 4 bytes of the directory entry are the file size.

The file size is merely the number of bytes in the file. You will still have to follow the cluster chain for reads that are not in the first cluster or that cross a cluster boundary.

I highly suggest you read the "fatgen130.doc" file that you can find on the internet. It's the definitive specification for the FAT filesystem, and it's all you should need to implement a FAT driver. My FAT driver supports FAT12, FAT16 and FAT32; you may find it beneficial to read the code and compare it to the spec to try and understand what's happening.

However, it does look like you have some bad practices in your code...

Code: Select all

unsigned char *buff = 0;

You really should be either:

• Passing the buffer to read into to fread (to avoid overwriting other concurrent reads), or
• Allocate some memory (malloc, if you have it, or use a static sized array)

I know you just want to get it working, but ignoring good coding practices to do so is going to bite you later. The first option is something like:

Code: Select all

void* _cdecl fread (unsigned char *fp, unsigned int offset, unsigned int count, bool readasint, char *buffer)
{
    /** Read <count> bytes into <buffer>, starting at <offset> bytes into the file **/
    return buffer;
}

Finally,

Code: Select all

char *testfname;

Again, testfname isn't defined... It could be pointing to code in your kernel for all you know, and you'll end up overwriting valid code with the filename.

The best course of action for you now is to fix the problems with your code. Once you're no longer reading from null or undefined pointers, you'll be in a better place to debug the problems with your FAT driver.

Cheers,
Matt

[accelleon]

The fatgen103 is the one I was using... As for the fread buff variable, I tend to program till I literally can't think about anything but sleep over the weekends; I will fix that now but will not have that as a parameter until I get some working code or if that's the only error. As for the testfname I fixed that as soon as I read neon's post . As for the cluster following replacement I converted the size to the number of sectors held by dividing by the bytes per sector, considering that in my boot sector each cluster is one sector. Anyway off to initializing every variable correctly.

[pcmattman]

Hi,

As for the cluster following replacement I converted the size to the number of sectors held by dividing by the bytes per sector, considering that in my boot sector each cluster is one sector.

Regardless, you cannot ever assume that the cluster layout on disk is contiguous. If a read will pass a cluster boundary, you must follow the chain in the FAT - there's no getting around it.

That'll also give you an off-by-one error: if a file is 516 bytes, 516 / 512 = 1, not 2 (truncated).

As for the fread buff variable, I tend to program till I literally can't think about anything but sleep over the weekends

If you're spending that much time coding, then I don't see why you can't just take the extra minute or so to think about what you're writing and do it right the first time...

Cheers,
Matt

[accelleon]

Yeah I guess there is no excuse, but regardless fixing your own mistakes is just one way off learning more, so at least I learned something today . Any way back to finishing my LoadFAT function then implement the cluster following code.

[Creature]

Code: Select all

void* _cdecl fread (unsigned char *fp, unsigned int offset, unsigned int count, bool readasint)
{
	unsigned char *buff = 0;
	for(int i=0; i<count; i++)
	{
		buff[i] = fp[offset + count]; // if not in virtual memory, say good bye to the IVT

If you thought that then you'd be surprised to know that that code actually worked fine . (for once in my FAT12 driver )

It probably works fine because you're probably writing to some random address in memory that isn't used at that time. It's like doing this:

Code: Select all

unsigned *p = 0x123456; // I need somewhere to store my value and this address is 'usable' memory.
*p = 5; // Sure, the value at this address might become 5, but if e.g. the heap had a header there, it will be corrupted and your OS will break down.

I suggest you either use a static array (if you know the size of the array before-hand and it is small), such as

Code: Select all

byte NameBuffer[11];

which can be destroyed afterwards, or use some kind of dynamic memory allocation.

[accelleon]

I would need to use more an array more than 1024: 512 for each sector and due to the fact that the FAT and the Root Directory span over multiple sectors plus I would then need to use an array like that for every file I load into memory. I could just check which memory is free in the memory manager, load the FAT, Root Directory or file to the memory then mark it as used. (If possible)

[Combuster]

Might I recommend passing /W4 /Wall /Wp64 /WX to the visual studio compiler. It might magically solve your problems (and more )

(disclaimer: I don't know your VC version so some switches might be different in your version)

[accelleon]

I am using VC++ 2005 for this project but I am will use VC++ 2008 for my OS.