Discussion Forum

Whats wrong with this code?

Next Thread | Thread List | Previous Thread Start a Thread | Settings

DetailsMessage
Read-Only
Author
almak sreem
Posted
3-Jul-2007 09:31 GMT
Toolset
None
New! Whats wrong with this code?

unsigned char sectors_per_cluster;
unsigned int *pi;
unsigned long *pl;
pi = (unsigned int *)&raw_block[FAT_BYTES_PER_SECTOR];
partition.bytes_per_sector = *pi++;
partition.sectors_per_cluster = *pi & 0xFF;
pi = (unsigned int *)&raw_block[FAT_RESERVED_SECTORS];
reserved_sectors = *pi++;
number_of_fats = *pi & 0xFF;
pl = (unsigned long *)&raw_block[FAT_FAT_SIZE];
sectors_per_fat = *pl;
partition.fat1_start_sector = partition.offset + reserved_sectors;
partition.fat2_start_sector = partition.fat1_start_sector + sectors_per_fat;
partition.root_dir_start_sector = partition.fat2_start_sector + sectors_per_fat;
partition.last_free_chain = 0;
fat_find_free_chain();}
static unsigned long fat_calc_data_address(unsigned long cluster, unsigned char sector)
{unsigned long offset;
offset = ((cluster - 2) * partition.sectors_per_cluster);
offset += partition.root_dir_start_sector;
offset += sector;
offset *= partition.bytes_per_sector;
return offset;}
static unsigned long fat_calc_chain_address(unsigned long index)
{unsigned long offset;
offset = partition.fat1_start_sector + (index >> 7);
offset *= partition.bytes_per_sector;
return offset;}
void fat_load_root_directory(void)
{unsigned long offset;
offset = partition.root_dir_start_sector;
offset *= partition.bytes_per_sector;
mmc_read(offset);
directory.parent_cluster = 0x00;
directory.start_cluster = 0x02;
directory.current_sector = 0x00;
directory.current_cluster = 0x02;
directory.current_direntry = (DIRENTRY *)FIRST_ENTRY_IN_ROOT;}
static void fat_load_directory(unsigned long cluster, unsigned char sector, unsigned char new)
{mmc_read(fat_calc_data_address(cluster, sector));
if (new)
{directory.parent_cluster = directory.start_cluster;
directory.start_cluster = cluster;
directory.current_direntry = (DIRENTRY *)FIRST_ENTRY_IN_DIR;}
directory.current_cluster = cluster;
directory.current_sector = sector;}
static void fat_read_next_sector_or_cluster(void)
{if (directory.current_sector == (partition.sectors_per_cluster-1))
fat_load_directory(fat_next_chain(directory.current_cluster), 0, FALSE);
else
{directory.current_sector++;
fat_load_directory(directory.current_cluster, directory.current_sector, FALSE);}}
static DIRENTRY *fat_seek_directory(char *name, unsigned char find_free)
{unsigned char i;
DIRENTRY *p = (DIRENTRY *)START_OF_SECTOR;
fat_load_directory(directory.start_cluster, 0, FALSE);
i = 0;
while(1)
{i++;
if (match_name(name, p->Name, 8))
if (p->Attribute & ATTRIB_SUBDIR)
{if (!find_free) return p;
else return 0;}
if (!p->Name[0])
{if (!find_free)
return 0;
else
return p;}
p++;
if (!(i % 0x10))
{i = 0;
p = (DIRENTRY *)START_OF_SECTOR;
fat_read_next_sector_or_cluster();}}
return 0;}
static DIRENTRY *fat_seek_file(char *name, unsigned char find_free)
{unsigned char i;
DIRENTRY *p = (DIRENTRY *)START_OF_SECTOR;
fat_load_directory(directory.start_cluster, 0, FALSE);
i = 0;
while(1)
{i++;
if (match_name(name, p->Name, 11))
if (p->Attribute == ATTRIB_ARCHIVE)
return p;
if (!p->Name[0])
{if (!find_free) return 0;
else return p;}
p++;
if (!(i % 0x10))
{i = 0;
p = (DIRENTRY *)START_OF_SECTOR;
fat_read_next_sector_or_cluster();}}
return 0;}
static unsigned long fat_find_free_chain(void)
{unsigned long index = partition.last_free_chain;
unsigned long *p;
mmc_read(fat_calc_chain_address(index));
p = (unsigned long* )START_OF_SECTOR;
p += (index & 0x7F);
while(*p)
{index++;
p++;
if (!(index % 0x80))
{mmc_read(fat_calc_chain_address(index));
p = (unsigned long* )START_OF_SECTOR;}}
partition.last_free_chain = index;
return index;}
unsigned char fat_read_direntry(unsigned char first)
{if (first)
fat_load_directory(directory.start_cluster, 0, FALSE);
else
{if (directory.current_direntry == (DIRENTRY *)LAST_DIRENTRY)
{fat_read_next_sector_or_cluster();
directory.current_direntry = (DIRENTRY *)START_OF_SECTOR;}
else
directory.current_direntry += 0x20; }}
static unsigned long fat_next_chain(unsigned long from)
{unsigned long *p;
mmc_read(fat_calc_chain_address(from));
p = (unsigned long* )START_OF_SECTOR;
p += (from & 0x7F);
return *p;}
void fat_mark_next_chain(unsigned long old, unsigned long new)
{unsigned long *p;
unsigned long offset;
offset = fat_calc_chain_address(old);
mmc_read(offset);
p = (unsigned long* )START_OF_SECTOR;
p += (old & 0x7F);
*p = new;
mmc_write(offset);
offset = fat_calc_chain_address(new);
mmc_read(offset);
p = (unsigned long* )START_OF_SECTOR;
p += (new & 0x7F);
*p = 0x0FFFFFFF;
mmc_write(offset);}
void fat_chain_free(unsigned long index)
{unsigned long *p;
unsigned long temp = index;
unsigned long offset;
while (temp)
{offset = fat_calc_chain_address(temp);
mmc_read(offset);
p = (unsigned long* )START_OF_SECTOR;
p += (temp & 0x7F);
temp = *p;
*p = 0;
mmc_write(offset); }}
static void fat_clear_sector(unsigned long cluster, unsigned char sector)
{mmc_clear(fat_calc_data_address(cluster, sector));}
unsigned char fat_make_directory(char *name)

Read-Only
Author
Andy Neil
Posted
3-Jul-2007 09:43 GMT
Toolset
None
New! Many things!

You didn't read and/or follow the instructions for posting source code - so it's illegible.

You haven't said what toolset is applicable.

You haven't said what it's supposed to do, and it appears to contain no comments.

You haven't said why you think it's "wrong" - what does it do that it shouldn't, and/or what doesn't it do that it should?

Read-Only
Author
almak sreem
Posted
3-Jul-2007 09:50 GMT
Toolset
C251
New! What's wrong with this code number 2

C251
Code is for fat on a SD memory stick
I use self documented code so comments are not needed
When I open a file I do not get the right data

unsigned char sectors_per_cluster;
unsigned int *pi;
unsigned long *pl;
pi = (unsigned int *)&raw_block[FAT_BYTES_PER_SECTOR];
partition.bytes_per_sector = *pi++;
partition.sectors_per_cluster = *pi & 0xFF;
pi = (unsigned int *)&raw_block[FAT_RESERVED_SECTORS];
reserved_sectors = *pi++;
number_of_fats = *pi & 0xFF;
pl = (unsigned long *)&raw_block[FAT_FAT_SIZE];
sectors_per_fat = *pl;
partition.fat1_start_sector = partition.offset + reserved_sectors;
partition.fat2_start_sector = partition.fat1_start_sector + sectors_per_fat;
partition.root_dir_start_sector = partition.fat2_start_sector + sectors_per_fat;
partition.last_free_chain = 0;
fat_find_free_chain();}
static unsigned long fat_calc_data_address(unsigned long cluster, unsigned char sector)
{unsigned long offset;
offset = ((cluster - 2) * partition.sectors_per_cluster);
offset += partition.root_dir_start_sector;
offset += sector;
offset *= partition.bytes_per_sector;
return offset;}
static unsigned long fat_calc_chain_address(unsigned long index)
{unsigned long offset;
offset = partition.fat1_start_sector + (index >> 7);
offset *= partition.bytes_per_sector;
return offset;}
void fat_load_root_directory(void)
{unsigned long offset;
offset = partition.root_dir_start_sector;
offset *= partition.bytes_per_sector;
mmc_read(offset);
directory.parent_cluster = 0x00;
directory.start_cluster = 0x02;
directory.current_sector = 0x00;
directory.current_cluster = 0x02;
directory.current_direntry = (DIRENTRY *)FIRST_ENTRY_IN_ROOT;}
static void fat_load_directory(unsigned long cluster, unsigned char sector, unsigned char new)
{mmc_read(fat_calc_data_address(cluster, sector));
if (new)
{directory.parent_cluster = directory.start_cluster;
directory.start_cluster = cluster;
directory.current_direntry = (DIRENTRY *)FIRST_ENTRY_IN_DIR;}
directory.current_cluster = cluster;
directory.current_sector = sector;}
static void fat_read_next_sector_or_cluster(void)
{if (directory.current_sector == (partition.sectors_per_cluster-1))
fat_load_directory(fat_next_chain(directory.current_cluster), 0, FALSE);
else
{directory.current_sector++;
fat_load_directory(directory.current_cluster, directory.current_sector, FALSE);}}
static DIRENTRY *fat_seek_directory(char *name, unsigned char find_free)
{unsigned char i;
DIRENTRY *p = (DIRENTRY *)START_OF_SECTOR;
fat_load_directory(directory.start_cluster, 0, FALSE);
i = 0;
while(1)
{i++;
if (match_name(name, p->Name, 8))
if (p->Attribute & ATTRIB_SUBDIR)
{if (!find_free) return p;
else return 0;}
if (!p->Name[0])
{if (!find_free)
return 0;
else
return p;}
p++;
if (!(i % 0x10))
{i = 0;
p = (DIRENTRY *)START_OF_SECTOR;
fat_read_next_sector_or_cluster();}}
return 0;}
static DIRENTRY *fat_seek_file(char *name, unsigned char find_free)
{unsigned char i;
DIRENTRY *p = (DIRENTRY *)START_OF_SECTOR;
fat_load_directory(directory.start_cluster, 0, FALSE);
i = 0;
while(1)
{i++;
if (match_name(name, p->Name, 11))
if (p->Attribute == ATTRIB_ARCHIVE)
return p;
if (!p->Name[0])
{if (!find_free) return 0;
else return p;}
p++;
if (!(i % 0x10))
{i = 0;
p = (DIRENTRY *)START_OF_SECTOR;
fat_read_next_sector_or_cluster();}}
return 0;}
static unsigned long fat_find_free_chain(void)
{unsigned long index = partition.last_free_chain;
unsigned long *p;
mmc_read(fat_calc_chain_address(index));
p = (unsigned long* )START_OF_SECTOR;
p += (index & 0x7F);
while(*p)
{index++;
p++;
if (!(index % 0x80))
{mmc_read(fat_calc_chain_address(index));
p = (unsigned long* )START_OF_SECTOR;}}
partition.last_free_chain = index;
return index;}
unsigned char fat_read_direntry(unsigned char first)
{if (first)
fat_load_directory(directory.start_cluster, 0, FALSE);
else
{if (directory.current_direntry == (DIRENTRY *)LAST_DIRENTRY)
{fat_read_next_sector_or_cluster();
directory.current_direntry = (DIRENTRY *)START_OF_SECTOR;}
else
directory.current_direntry += 0x20; }}
static unsigned long fat_next_chain(unsigned long from)
{unsigned long *p;
mmc_read(fat_calc_chain_address(from));
p = (unsigned long* )START_OF_SECTOR;
p += (from & 0x7F);
return *p;}
void fat_mark_next_chain(unsigned long old, unsigned long new)
{unsigned long *p;
unsigned long offset;
offset = fat_calc_chain_address(old);
mmc_read(offset);
p = (unsigned long* )START_OF_SECTOR;
p += (old & 0x7F);
*p = new;
mmc_write(offset);
offset = fat_calc_chain_address(new);
mmc_read(offset);
p = (unsigned long* )START_OF_SECTOR;
p += (new & 0x7F);
*p = 0x0FFFFFFF;
mmc_write(offset);}
void fat_chain_free(unsigned long index)
{unsigned long *p;
unsigned long temp = index;
unsigned long offset;
while (temp)
{offset = fat_calc_chain_address(temp);
mmc_read(offset);
p = (unsigned long* )START_OF_SECTOR;
p += (temp & 0x7F);
temp = *p;
*p = 0;
mmc_write(offset); }}
static void fat_clear_sector(unsigned long cluster, unsigned char sector)
{mmc_clear(fat_calc_data_address(cluster, sector));}
unsigned char fat_make_directory(char *name)
Read-Only
Author
David Rose
Posted
3-Jul-2007 10:01 GMT
Toolset
C251
New! RE: What's wrong with this code number 2

I use self documented code so comments are not needed

If this is an example of self-documented code, then I think I'll stick with the old-school method of including comments!

What about indentation?

Also looks like you've missed the start of the first function. Maybe it's important? Probably would stop the code compiling.

Oh - And looks like the end of the last function is missing too.

Please consider tidying up the code before posting again.

Read-Only
Author
almak sreem
Posted
3-Jul-2007 13:53 GMT
Toolset
C251
New! RE: What's wrong with this code number 3

Tidied code

void fatcheck(vodi)
{
unsigned char sectors_per_cluster;
unsigned int *pi;
unsigned long *pl;
pi = (unsigned int *)&raw_block[FAT_BYTES_PER_SECTOR];
partition.bytes_per_sector = *pi++;
partition.sectors_per_cluster = *pi & 0xFF;
pi = (unsigned int *)&raw_block[FAT_RESERVED_SECTORS];
reserved_sectors = *pi++;
number_of_fats = *pi & 0xFF;
pl = (unsigned long *)&raw_block[FAT_FAT_SIZE];
sectors_per_fat = *pl;
partition.fat1_start_sector = partition.offset + reserved_sectors;
partition.fat2_start_sector = partition.fat1_start_sector + sectors_per_fat;
partition.root_dir_start_sector = partition.fat2_start_sector + sectors_per_fat;
partition.last_free_chain = 0;
fat_find_free_chain();}
static unsigned long fat_calc_data_address(unsigned long cluster, unsigned char sector)
{unsigned long offset;
offset = ((cluster - 2) * partition.sectors_per_cluster);
offset += partition.root_dir_start_sector;
offset += sector;
offset *= partition.bytes_per_sector;
return offset;}
static unsigned long fat_calc_chain_address(unsigned long index)
{unsigned long offset;
offset = partition.fat1_start_sector + (index >> 7);
offset *= partition.bytes_per_sector;
return offset;}
void fat_load_root_directory(void)
{unsigned long offset;
offset = partition.root_dir_start_sector;
offset *= partition.bytes_per_sector;
mmc_read(offset);
directory.parent_cluster = 0x00;
directory.start_cluster = 0x02;
directory.current_sector = 0x00;
directory.current_cluster = 0x02;
directory.current_direntry = (DIRENTRY *)FIRST_ENTRY_IN_ROOT;}
static void fat_load_directory(unsigned long cluster, unsigned char sector, unsigned char new)
{mmc_read(fat_calc_data_address(cluster, sector));
if (new)
{directory.parent_cluster = directory.start_cluster;
directory.start_cluster = cluster;
directory.current_direntry = (DIRENTRY *)FIRST_ENTRY_IN_DIR;}
directory.current_cluster = cluster;
directory.current_sector = sector;}
static void fat_read_next_sector_or_cluster(void)
{if (directory.current_sector == (partition.sectors_per_cluster-1))
fat_load_directory(fat_next_chain(directory.current_cluster), 0, FALSE);
else
{directory.current_sector++;
fat_load_directory(directory.current_cluster, directory.current_sector, FALSE);}}
static DIRENTRY *fat_seek_directory(char *name, unsigned char find_free)
{unsigned char i;
DIRENTRY *p = (DIRENTRY *)START_OF_SECTOR;
fat_load_directory(directory.start_cluster, 0, FALSE);
i = 0;
while(1)
{i++;
if (match_name(name, p->Name, 8))
if (p->Attribute & ATTRIB_SUBDIR)
{if (!find_free) return p;
else return 0;}
if (!p->Name[0])
{if (!find_free)
return 0;
else
return p;}
p++;
if (!(i % 0x10))
{i = 0;
p = (DIRENTRY *)START_OF_SECTOR;
fat_read_next_sector_or_cluster();}}
return 0;}
static DIRENTRY *fat_seek_file(char *name, unsigned char find_free)
{unsigned char i;
DIRENTRY *p = (DIRENTRY *)START_OF_SECTOR;
fat_load_directory(directory.start_cluster, 0, FALSE);
i = 0;
while(1)
{i++;
if (match_name(name, p->Name, 11))
if (p->Attribute == ATTRIB_ARCHIVE)
return p;
if (!p->Name[0])
{if (!find_free) return 0;
else return p;}
p++;
if (!(i % 0x10))
{i = 0;
p = (DIRENTRY *)START_OF_SECTOR;
fat_read_next_sector_or_cluster();}}
return 0;}
static unsigned long fat_find_free_chain(void)
{unsigned long index = partition.last_free_chain;
unsigned long *p;
mmc_read(fat_calc_chain_address(index));
p = (unsigned long* )START_OF_SECTOR;
p += (index & 0x7F);
while(*p)
{index++;
p++;
if (!(index % 0x80))
{mmc_read(fat_calc_chain_address(index));
p = (unsigned long* )START_OF_SECTOR;}}
partition.last_free_chain = index;
return index;}
unsigned char fat_read_direntry(unsigned char first)
{if (first)
fat_load_directory(directory.start_cluster, 0, FALSE);
else
{if (directory.current_direntry == (DIRENTRY *)LAST_DIRENTRY)
{fat_read_next_sector_or_cluster();
directory.current_direntry = (DIRENTRY *)START_OF_SECTOR;}
else
directory.current_direntry += 0x20; }}
static unsigned long fat_next_chain(unsigned long from)
{unsigned long *p;
mmc_read(fat_calc_chain_address(from));
p = (unsigned long* )START_OF_SECTOR;
p += (from & 0x7F);
return *p;}
void fat_mark_next_chain(unsigned long old, unsigned long new)
{unsigned long *p;
unsigned long offset;
offset = fat_calc_chain_address(old);
mmc_read(offset);
p = (unsigned long* )START_OF_SECTOR;
p += (old & 0x7F);
*p = new;
mmc_write(offset);
offset = fat_calc_chain_address(new);
mmc_read(offset);
p = (unsigned long* )START_OF_SECTOR;
p += (new & 0x7F);
*p = 0x0FFFFFFF;
mmc_write(offset);}
void fat_chain_free(unsigned long index)
{unsigned long *p;
unsigned long temp = index;
unsigned long offset;
while (temp)
{offset = fat_calc_chain_address(temp);
mmc_read(offset);
p = (unsigned long* )START_OF_SECTOR;
p += (temp & 0x7F);
temp = *p;
*p = 0;
mmc_write(offset); }}
static void fat_clear_sector(unsigned long cluster, unsigned char sector)
{mmc_clear(fat_calc_data_address(cluster, sector));}
unsigned char fat_make_directory(char *name)
}
Read-Only
Author
Dan Henry
Posted
3-Jul-2007 14:13 GMT
Toolset
C251
New! RE: What's wrong with this code number 3

"Tidied code"

Nope. Try again. Preview next time. If you don't preserve formatting (indentation), nobody's going to give it a second look.

Read-Only
Author
Per Westermark
Posted
3-Jul-2007 15:37 GMT
Toolset
C251
New! RE: What's wrong with this code number 3

"self-documenting code" can tell you what a function or variable is used for. However, it seldom answers the question "why".

Read-Only
Author
erik malund
Posted
3-Jul-2007 15:46 GMT
Toolset
C251
New! RE: What's wrong with this code number 3

two sentences of equal validity:
"my code is self documenting"
"the moon is made of green cheese"

Erik

Read-Only
Author
Per Westermark
Posted
3-Jul-2007 15:51 GMT
Toolset
C251
New! RE: What's wrong with this code number 3

Code _can_ be self-documenting. It's a question of complexity compared to the expected knowledge of the reader.

A trivial example is a "hello world" app. If that isn't self-documenting, the reader should switch to doing something else.

The big problem is always to figure out what a suitable level of documentation is, since the world is never black _or_ white.

Read-Only
Author
erik malund
Posted
3-Jul-2007 16:18 GMT
Toolset
C251
New! RE: What's wrong with this code number 3

The big problem is always to figure out what a suitable level of documentation is, since the world is never black _or_ white.
agree, to some extent; however, I would never complain that any code is too commented.

A trivial example is a "hello world" app. If that isn't self-documenting, the reader should switch to doing something else.
This is the road to perdition, where does "self-documenting" end?. "hello world" is NOT "self-documenting"; however it is so simple that the lack of documentation does not limit the understanmdin of the process.

So, instead of the blasted untrue "self-documenting" can we agree on "so simple that documenting not needed"

Using that instead of the blasted "self-documenting" the OPs argument "commrnts not needed" falls flat.

Erik

Read-Only
Author
Hans-Bernhard Broeker
Posted
3-Jul-2007 22:45 GMT
Toolset
C251
New! RE: What's wrong with this code number 3

I would never complain that any code is too commented.

I would. As the first of the ancient pharmacologists noted, dosage is what makes a poison. People will take sayings like "you can never have too much comment" literally, and you end up with a code/comment ratio of 1 percent or less --- one line of code per screenful of source. If you can't see more than three lines of actual code at a glance on a full-screen editor, then yes, that means the code is over-commented.

Read-Only
Author
Andy Neil
Posted
3-Jul-2007 19:26 GMT
Toolset
C251
New! RE: What's wrong with this code number 3

"Tidied code"

Hmmm... this is obviously some new meaning of the word "tidy" of which I was previously unaware!

Or, perhaps this is "tidy" in the sense that a teenager might consider their room to be "tidy"...?!

"I use self documented code"

Again, if you consider that "self documented", I should hate to see what your obfuscated code would look like!!

Identifiers like 'pi' and 'pl' are hardly meaningful;
Magic numbers like 0x10, 0x0FFFFFFF, etc are certainly not "self documenting"

etc, etc...

Read-Only
Author
erik malund
Posted
3-Jul-2007 19:58 GMT
Toolset
C251
New! RE: What's wrong with this code number 3

this is obviously some new meaning of the word "tidy"

I thought I would help by illustrating formatting by taking a snippet of the OPs code and format it. Lo and behold, I could not even see where to begin.

If your kitchen was 'tidy' like that, the tidiness would be self-documented by cockroaches.

Erik

Read-Only
Author
Andy Neil
Posted
3-Jul-2007 20:39 GMT
Toolset
C251
New! Test case?

"I thought I would help by illustrating formatting by taking a snippet of the OPs code and format it. Lo and behold, I could not even see where to begin."

There are tools that are supposed to be able to automatically format code.
Maybe this could be used as a test case...?
Or should I say, "a challenge"?

Of course, it doesn't help that two of the posts are obviously incomplete.

Read-Only
Author
erik malund
Posted
3-Jul-2007 20:42 GMT
Toolset
C251
New! I'm impressed

Of course, it doesn't help that two of the posts are obviously incomplete

I'm impressed that you could decipher the scribbles far enough to even see that.

Erik

Read-Only
Author
Per Westermark
Posted
3-Jul-2007 21:07 GMT
Toolset
C251
New! RE: I'm impressed

agree, to some extent; however, I would never complain that any code is too commented.

Well, there are two ways of abusing comments. One is to not write any comments even if the code is non-obvious, or it isn't even obvious why the code needs to be run.

The other abuse is people who think they have to document how the language works.

Ever seen:

mov al,0    ; assign 0 to al
mov bl,al   ; copy al to bl
...

or

score = score + 1;       // count up score
if (score > highscore)   // check if larger than highscore
    highscore = score;   // set highscore to score
...

Having the code full of obvious comments is about as funny as having code where every single precedence rule is overriden by lisp-style parentheses. Too much noise makes code bothersome too.

If the indenting is then random, and the comments are inlined at random positions, you really have to scan bast the comments just to figure out where code blocks starts/ends. With too weird positions of comments, even indent gets a hard time making cleaning up.

Read-Only
Author
erik malund
Posted
3-Jul-2007 21:25 GMT
Toolset
C251
New! not comments, but explanations

Ever seen:

mov al,0 ; assign 0 to al
mov bl,al ; copy al to bl
...

or

score = score + 1; // count up score
if (score > highscore) // check if larger than highscore highscore = score; // set highscore to score
...

those are not not comments, but explanations; However, it is good you cleared that one up

If the indenting is then random, and the comments are inlined at random positions, you really have to scan bast the comments just to figure out where code blocks starts/ends. With too weird positions of comments, even indent gets a hard time making cleaning up.
in other words "if you can't read the code, comments do not help" - agreed.

Erik

Read-Only
Author
Neil Kurzman
Posted
4-Jul-2007 04:51 GMT
Toolset
C251
New! RE: What's wrong with this code number 3

Exclusive
Of the Code is poorly formated and Not Commented.

Why is it Here? Does it compile? Any warnings or errors? Is the Warning level set High? What line?
If it does not work what kind of problem?

You must think very highly of other programmers that they could just see the if code they can not run.

Read-Only
Author
Andy Neil
Posted
4-Jul-2007 06:57 GMT
Toolset
C251
New! RE: What's wrong with this code number 4

"Does it compile?"

Clrearly not!

For a start, what is "vodi"?

And, as you say, there's (at least) on bracket missing...

Global definitions are also missing; eg, raw_block, partition, sectors_per_fat, reserved_sectors, number_of_fats,...

Read-Only
Author
Neil Kurzman
Posted
4-Jul-2007 05:12 GMT
Toolset
C251
New! RE: What's wrong with this code number 3
void fatcheck(vodi)
{
   unsigned char sectors_per_cluster;
   unsigned int *pi;
   unsigned long *pl;

   pi = (unsigned int *)&raw_block[FAT_BYTES_PER_SECTOR];
   partition.bytes_per_sector = *pi++;
   partition.sectors_per_cluster = *pi & 0xFF;
   pi = (unsigned int *)&raw_block[FAT_RESERVED_SECTORS];
   reserved_sectors = *pi++;
   number_of_fats = *pi & 0xFF;
   pl = (unsigned long *)&raw_block[FAT_FAT_SIZE];
   sectors_per_fat = *pl;
   partition.fat1_start_sector = partition.offset +   reserved_sectors;
   partition.fat2_start_sector =    partition.fat1_start_sector + sectors_per_fat;
   partition.root_dir_start_sector =   partition.fat2_start_sector + sectors_per_fat;
   partition.last_free_chain = 0;
   fat_find_free_chain();
}

static unsigned long fat_calc_data_address(unsigned long cluster, unsigned char sector)
{
   unsigned long offset;

   offset = ((cluster - 2) *    partition.sectors_per_cluster);
   offset += partition.root_dir_start_sector;
   offset += sector;
   offset *= partition.bytes_per_sector;
   return offset;
}

static unsigned long fat_calc_chain_address(unsigned long index)
{
   unsigned long offset;

   offset = partition.fat1_start_sector + (index >> 7);
   offset *= partition.bytes_per_sector;
   return offset;
}

void fat_load_root_directory(void)
{
   unsigned long offset;

   offset = partition.root_dir_start_sector;
   offset *= partition.bytes_per_sector;
   mmc_read(offset);
   directory.parent_cluster = 0x00;
   directory.start_cluster = 0x02;
   directory.current_sector = 0x00;
   directory.current_cluster = 0x02;
   directory.current_direntry = (DIRENTRY *)FIRST_ENTRY_IN_ROOT;
}

static void fat_load_directory(unsigned long cluster, unsigned char sector, unsigned char new)
{
   mmc_read(fat_calc_data_address(cluster, sector));
   if (new)
   {
      directory.parent_cluster =  directory.start_cluster;
      directory.start_cluster = cluster;
      directory.current_direntry = (DIRENTRY  *)FIRST_ENTRY_IN_DIR;}
      directory.current_cluster = cluster;
      directory.current_sector = sector;
   }

//MISSING A BRACKET HERE

static void fat_read_next_sector_or_cluster(void)
{
   if (directory.current_sector ==  (partition.sectors_per_cluster-1))
   {
      fat_load_directory(fat_next_chain(directory.current_cluster), 0, FALSE);
   }
   else
   {
      directory.current_sector++;
      fat_load_directory(directory.current_cluster,    directory.current_sector, FALSE);
   }
}

static DIRENTRY *fat_seek_directory(char *name, unsigned char find_free)
{
   unsigned char i;

   DIRENTRY *p = (DIRENTRY *)START_OF_SECTOR;
   fat_load_directory(directory.start_cluster, 0, FALSE);
   i = 0;
   while(1)
   {
      i++;
      if (match_name(name, p->Name, 8))
      if (p->Attribute & ATTRIB_SUBDIR)
      {
         if (!find_free) return p;
         else return 0;
      }
      if (!p->Name[0])
      {
         if (!find_free)
            return 0;
         else
            return p;
      }
      p++;
      if (!(i % 0x10))
      {
         i = 0;
         p = (DIRENTRY *)START_OF_SECTOR;
         fat_read_next_sector_or_cluster();
      }
   }
   return 0;
}

static DIRENTRY *fat_seek_file(char *name, unsigned char find_free)
{
   unsigned char i;
   DIRENTRY *p = (DIRENTRY *)START_OF_SECTOR;
   fat_load_directory(directory.start_cluster, 0,   FALSE);
   i = 0;
   while(1)
   {
      i++;
      if (match_name(name, p->Name, 11))
      if (p->Attribute == ATTRIB_ARCHIVE)
         return p;
      if (!p->Name[0])
      {
         if (!find_free) return 0;
         else return p;
      }
      p++;
      if (!(i % 0x10))
      {
         i = 0;
         p = (DIRENTRY *)START_OF_SECTOR;
         fat_read_next_sector_or_cluster();
      }
   }
   return 0;
}

static unsigned long fat_find_free_chain(void)
{
   unsigned long index = partition.last_free_chain;
   unsigned long *p;

   mmc_read(fat_calc_chain_address(index));
   p = (unsigned long* )START_OF_SECTOR;
   p += (index & 0x7F);
   while(*p)
   {
      index++;
      p++;
      if (!(index % 0x80))
      {
         mmc_read(fat_calc_chain_address(index));
         p = (unsigned long* )START_OF_SECTOR;
      }
   }
   partition.last_free_chain = index;
   return index;
}

unsigned char fat_read_direntry(unsigned char first)
{
   if (first)
   {
      fat_load_directory(directory.start_cluster, 0,  FALSE);
   }
   else
   {
      if (directory.current_direntry == (DIRENTRY *)LAST_DIRENTRY)
      {
         fat_read_next_sector_or_cluster();
         directory.current_direntry = (DIRENTRY *)START_OF_SECTOR;
      }
      else
      {
         directory.current_direntry += 0x20;
      }
   }
}

static unsigned long fat_next_chain(unsigned long from)
{
   unsigned long *p;

   mmc_read(fat_calc_chain_address(from));
   p = (unsigned long* )START_OF_SECTOR;
   p += (from & 0x7F);
   return *p;
}

void fat_mark_next_chain(unsigned long old, unsigned long new)
{
   unsigned long *p;
   unsigned long offset;

   offset = fat_calc_chain_address(old);
   mmc_read(offset);
   p = (unsigned long* )START_OF_SECTOR;
   p += (old & 0x7F);
   *p = new;
   mmc_write(offset);
   offset = fat_calc_chain_address(new);
   mmc_read(offset);
   p = (unsigned long* )START_OF_SECTOR;
   p += (new & 0x7F);
   *p = 0x0FFFFFFF;
   mmc_write(offset);
}

void fat_chain_free(unsigned long index)
{
   unsigned long *p;
   unsigned long temp = index;
   unsigned long offset;

   while (temp)
   {
      offset = fat_calc_chain_address(temp);
      mmc_read(offset);
      p = (unsigned long* )START_OF_SECTOR;
      p += (temp & 0x7F);
      temp = *p;
      *p = 0;
      mmc_write(offset);
   }
}

static void fat_clear_sector(unsigned long cluster, unsigned char sector)
{
   mmc_clear(fat_calc_data_address(cluster, sector));
}

unsigned char fat_make_directory(char *name)
{
}

You seem to have a lot of unmatch brackets,  I think?
White space is free, use some.  Code is not just for you.  Others may have to read it.


Read-Only
Author
almak sreem
Posted
4-Jul-2007 08:59 GMT
Toolset
C251
New! RE: What's wrong with this code number 3

The code compiles, but I had to give only the important bits because the forum does not like big posts.

I know varbials are not in my listing because they will not fit in the message.

vodi is a typing mistake and is void in the code of course.

Give me you're email address now and I will send you the code to fix.

thank you.

Read-Only
Author
Andy Neil
Posted
4-Jul-2007 10:36 GMT
Toolset
C251
New! RE: What's wrong with this code number 3

"vodi is a typing mistake"

Which is why you should never re-type the code into the forum! Always use copy-and-paste!

"and is void in the code of course"

Why of course?
All you've said so far is that it doesn't work - despite being asked, you haven't actually confirmed yet that it compiles cleanly (ie, without errors or warnings).

And if that's one obvious typing mistake, how many more are buried in your post?!

"Give me you're email address now and I will send you the code to fix."

Well, if you sent the complete project (not just a few source files) together with a thorough description of what it's supposed to do, the target hardware it's supposed to run on, and how exactly it is failing, someone might be prepared to give you a Quote to fix it for you...

Read-Only
Author
almak sreem
Posted
4-Jul-2007 10:57 GMT
Toolset
C251
New! RE: What's wrong with this code number 3

What is you're email address?

Read-Only
Author
almak sreem
Posted
4-Jul-2007 10:58 GMT
Toolset
C251
New! RE: What's wrong with this code number 3

I said the code compiles!

Read-Only
Author
almak sreem
Posted
5-Jul-2007 11:37 GMT
Toolset
C251
New! What's wrong with this code - Tidied up (more)

Whats' wrong with this code

void fatcheck(void)
{
   unsigned char sectors_per_cluster;
   unsigned int *pi;
   unsigned long *pl;

   pi = (unsigned int *)&raw_block[FAT_BYTES_PER_SECTOR];
   partition.bytes_per_sector = *pi++;
   partition.sectors_per_cluster = *pi & 0xFF;
   pi = (unsigned int *)&raw_block[FAT_RESERVED_SECTORS];
   reserved_sectors = *pi++;
   number_of_fats = *pi & 0xFF;
   pl = (unsigned long *)&raw_block[FAT_FAT_SIZE];
   sectors_per_fat = *pl;
   partition.fat1_start_sector = partition.offset +   reserved_sectors;
   partition.fat2_start_sector =    partition.fat1_start_sector + sectors_per_fat;
   partition.root_dir_start_sector =   partition.fat2_start_sector + sectors_per_fat;
   partition.last_free_chain = 0;
   fat_find_free_chain();
}

static unsigned long fat_calc_data_address(unsigned long cluster, unsigned char sector)
{
   unsigned long offset;

   offset = ((cluster - 2) *    partition.sectors_per_cluster);
   offset += partition.root_dir_start_sector;
   offset += sector;
   offset *= partition.bytes_per_sector;
   return offset;
}

static unsigned long fat_calc_chain_address(unsigned long index)
{
   unsigned long offset;

   offset = partition.fat1_start_sector + (index >> 7);
   offset *= partition.bytes_per_sector;
   return offset;
}

void fat_load_root_directory(void)
{
   unsigned long offset;

   offset = partition.root_dir_start_sector;
   offset *= partition.bytes_per_sector;
   mmc_read(offset);
   directory.parent_cluster = 0x00;
   directory.start_cluster = 0x02;
   directory.current_sector = 0x00;
   directory.current_cluster = 0x02;
   directory.current_direntry = (DIRENTRY *)FIRST_ENTRY_IN_ROOT;
}

static void fat_load_directory(unsigned long cluster, unsigned char sector, unsigned char new)
{
   mmc_read(fat_calc_data_address(cluster, sector));
   if (new)
   {
      directory.parent_cluster =  directory.start_cluster;
      directory.start_cluster = cluster;
      directory.current_direntry = (DIRENTRY  *)FIRST_ENTRY_IN_DIR;}
      directory.current_cluster = cluster;
      directory.current_sector = sector;
   }
}

static void fat_read_next_sector_or_cluster(void)
{
   if (directory.current_sector ==  (partition.sectors_per_cluster-1))
   {
      fat_load_directory(fat_next_chain(directory.current_cluster), 0, FALSE);
   }
   else
   {
      directory.current_sector++;
      fat_load_directory(directory.current_cluster,    directory.current_sector, FALSE);
   }
}

static DIRENTRY *fat_seek_directory(char *name, unsigned char find_free)
{
   unsigned char i;

   DIRENTRY *p = (DIRENTRY *)START_OF_SECTOR;
   fat_load_directory(directory.start_cluster, 0, FALSE);
   i = 0;
   while(1)
   {
      i++;
      if (match_name(name, p->Name, 8))
      if (p->Attribute & ATTRIB_SUBDIR)
      {
         if (!find_free) return p;
         else return 0;
      }
      if (!p->Name[0])
      {
         if (!find_free)
            return 0;
         else
            return p;
      }
      p++;
      if (!(i % 0x10))
      {
         i = 0;
         p = (DIRENTRY *)START_OF_SECTOR;
         fat_read_next_sector_or_cluster();
      }
   }
   return 0;
}

static DIRENTRY *fat_seek_file(char *name, unsigned char find_free)
{
   unsigned char i;
   DIRENTRY *p = (DIRENTRY *)START_OF_SECTOR;
   fat_load_directory(directory.start_cluster, 0,   FALSE);
   i = 0;
   while(1)
   {
      i++;
      if (match_name(name, p->Name, 11))
      if (p->Attribute == ATTRIB_ARCHIVE)
         return p;
      if (!p->Name[0])
      {
         if (!find_free) return 0;
         else return p;
      }
      p++;
      if (!(i % 0x10))
      {
         i = 0;
         p = (DIRENTRY *)START_OF_SECTOR;
         fat_read_next_sector_or_cluster();
      }
   }
   return 0;
}

static unsigned long fat_find_free_chain(void)
{
   unsigned long index = partition.last_free_chain;
   unsigned long *p;

   mmc_read(fat_calc_chain_address(index));
   p = (unsigned long* )START_OF_SECTOR;
   p += (index & 0x7F);
   while(*p)
   {
      index++;
      p++;
      if (!(index % 0x80))
      {
         mmc_read(fat_calc_chain_address(index));
         p = (unsigned long* )START_OF_SECTOR;
      }
   }
   partition.last_free_chain = index;
   return index;
}

unsigned char fat_read_direntry(unsigned char first)
{
   if (first)
   {
      fat_load_directory(directory.start_cluster, 0,  FALSE);
   }
   else
   {
      if (directory.current_direntry == (DIRENTRY *)LAST_DIRENTRY)
      {
         fat_read_next_sector_or_cluster();
         directory.current_direntry = (DIRENTRY *)START_OF_SECTOR;
      }
      else
      {
         directory.current_direntry += 0x20;
      }
   }
}

static unsigned long fat_next_chain(unsigned long from)
{
   unsigned long *p;

   mmc_read(fat_calc_chain_address(from));
   p = (unsigned long* )START_OF_SECTOR;
   p += (from & 0x7F);
   return *p;
}

void fat_mark_next_chain(unsigned long old, unsigned long new)
{
   unsigned long *p;
   unsigned long offset;

   offset = fat_calc_chain_address(old);
   mmc_read(offset);
   p = (unsigned long* )START_OF_SECTOR;
   p += (old & 0x7F);
   *p = new;
   mmc_write(offset);
   offset = fat_calc_chain_address(new);
   mmc_read(offset);
   p = (unsigned long* )START_OF_SECTOR;
   p += (new & 0x7F);
   *p = 0x0FFFFFFF;
   mmc_write(offset);
}

void fat_chain_free(unsigned long index)
{
   unsigned long *p;
   unsigned long temp = index;
   unsigned long offset;

   while (temp)
   {
      offset = fat_calc_chain_address(temp);
      mmc_read(offset);
      p = (unsigned long* )START_OF_SECTOR;
      p += (temp & 0x7F);
      temp = *p;
      *p = 0;
      mmc_write(offset);
   }
}

static void fat_clear_sector(unsigned long cluster, unsigned char sector)
{
   mmc_clear(fat_calc_data_address(cluster, sector));
}

unsigned char fat_make_directory(char *name)
{
   mmc_change(name);
}

Read-Only
Author
Andy Neil
Posted
5-Jul-2007 12:34 GMT
Toolset
C251
New! RE: What's wrong with this code - Tidied up (more)

"Whats' wrong with this code?"

It's red!

Read-Only
Author
Andy Neil
Posted
5-Jul-2007 12:39 GMT
Toolset
C251
New! RE: What's wrong with this code - Tidied up (more)

You still haven't answered: "...why you think it's "wrong" - what does it do that it shouldn't, and/or what doesn't it do that it should?"

You still haven't given any meaningful description of the "problems" that you've observed.

You still haven't even confirmed that it compiles without warnings.

Did you write this code yourself, or is it from a 3rd party?

What testing have you done in an attempt to narrow-down the source of these "problems"?

Have you tried it in a simulator?

Are you sure that the target is correct & working?

Are you sure that you've configured everything correctly?

Read-Only
Author
almak sreem
Posted
5-Jul-2007 13:01 GMT
Toolset
C251
New! RE: What's wrong with this code - Tidied up (more)

ok, its' red but that doesnt' stop it working ;)

You still haven't answered: "...why you think it's "wrong" - what does it do that it shouldn't, and/or what doesn't it do that it should?"

Whet I write to a file it looks good but when i read the file back its' f**ked up!

You still haven't given any meaningful description of the "problems" that you've observed.

See above.

You still haven't even confirmed that it compiles without warnings.

Yes it does.

Did you write this code yourself, or is it from a 3rd party?

Some I wrote myself and some from somewhere else.

What testing have you done in an attempt to narrow-down the source of these "problems"?

I know that the the problem is in the reading of a file, or the writing.

Have you tried it in a simulator?

Simulators are for wimps!

Are you sure that the target is correct & working?

Yes.

Are you sure that you've configured everything correctly?

Maybe a problem in the code.

Read-Only
Author
Andy Neil
Posted
5-Jul-2007 13:32 GMT
Toolset
C251
New! You need to put some work in to understand the problem

"Some I wrote myself and some from somewhere else."

Did the 3rd-party code work when you got it?

How did you check that it was suitable for your tools and your hardware?
eg, you make extensive use of int, long etc - are you sure that any assumptions about size, endianness, etc are valid?

What have you done to isolate whether the "problems" are in the bits you've changed, or the original 3rd-party bits?

"Whet I write to a file it looks good"

Where does it "look good"?
How are you checking it?
What are your criteria for "looking good" - does that mean you've carefully checked in detail that it's all 100.0% correct, or you've just had a quick glance?

"when i read the file back its' f**ked up!"

That's not very descriptive, is it?

How, exactly, is it "f**ked up"?

Where is it getting "f**ked up" - ie, are you low-level raw sector accesses working, or is it higher up the chain?

Can you read files that were created by other systems?

"Simulators are for wimps!"

And only idiots refuse to use a tool that could help them when they're stuck!
You're not getting anywhere without the simulator, so why not give it a try?
Why make life difficult for yourself?

Q: Are you sure that the target is correct & working?
A: Yes.

How are you sure?
What testing have you done?

Read-Only
Author
almak sreem
Posted
5-Jul-2007 13:59 GMT
Toolset
C251
New! RE: You need to put some work in to understand the problem

Some I wrote myself and some from somewhere else.
"Did the 3rd-party code work when you got it?

Yes, but not properly.

How did you check that it was suitable for your tools and your hardware?
eg, you make extensive use of int, long etc - are you sure that any assumptions about size, endianness, etc are valid?

The original code makes no such assumptions. I was told that it was platform and compiler neutral.

What have you done to isolate whether the "problems" are in the bits you've changed, or the original 3rd-party bits?

I put my bits into a separate file.

"Whet I write to a file it looks good"

Where does it "look good"?
How are you checking it?
What are your criteria for "looking good" - does that mean you've carefully checked in detail that it's all 100.0% correct, or you've just had a quick glance?

Yes. The call returns no error.

"when i read the file back its' f**ked up!"

That's not very descriptive, is it?

Pretty good description I though. Could also be decribed as scr**ed up or mu**ed up if you prefer.

How, exactly, is it "f**ked up"?

It doesnt do what it should. The read fails.

Where is it getting "f**ked up" - ie, are you low-level raw sector accesses working, or is it higher up the chain?

Interesting, I'll check on that one.

Can you read files that were created by other systems?

No, but the only other platform I use is Windows and I dont trust that. Do you?

"Simulators are for wimps!"

And only idiots refuse to use a tool that could help them when they're stuck!
You're not getting anywhere without the simulator, so why not give it a try?
Why make life difficult for yourself?

Id prefer to understand the code. Besides, how would I stimulate a memory card?

Q: Are you sure that the target is correct & working?
A: Yes.

How are you sure?
What testing have you done?

Erm, because the hardware guys said so. They tested it.

Read-Only
Author
Per Westermark
Posted
5-Jul-2007 14:56 GMT
Toolset
C251
New! RE: You need to put some work in to understand the problem

Simulators are for wimps!

If simulators are for wimps, then I would figure that asking for help in forums is also for wimps...

Have you done a raw dump of the written data, and manually checked that the data conforms to the expected format?

Read-Only
Author
almak sreem
Posted
5-Jul-2007 15:09 GMT
Toolset
C251
New! RE: You need to put some work in to understand the problem

If simulators are for wimps, then I would figure that asking for help in forums is also for wimps...

Following that logic ...

If all trees are green then it follows that all shoes are green, all cars are green and all g-strings are green.

Have you done a raw dump of the written data, and manually checked that the data conforms to the expected format?

Yes, but its complimercated and takes time!

Patience please.

Read-Only
Author
Andy Neil
Posted
5-Jul-2007 15:15 GMT
Toolset
C251
New! RE: You need to put some work in to understand the problem

"Yes, but its complimercated and takes time!"

So stop wasting your time making useless posts and get on with it!

Read-Only
Author
almak sreem
Posted
5-Jul-2007 15:32 GMT
Toolset
C251
New! RE: You need to put some work in to understand the problem

So stop wasting your time making useless posts and get on with it!

SIR, YES SIR

Read-Only
Author
Neil Kurzman
Posted
5-Jul-2007 20:55 GMT
Toolset
C251
New! RE: You need to put some work in to understand the problem

id prefer to understand the code. Besides, how would I stimulate a memory card?

Can you read a file put on the memory card by a PC?
Can your PC read a file you put on the memory card with a PC?

F**KUP says nothing. What do you read back Bad data? scrambled data? No data? Have you tried writing and reading a single byte? Does the file name appear on the card?

The chance that someone can just look at a few pages of code from an incomplete program and find a logic error are slim to none. AS you said none of use can run your code. the best anyone else can do is help you narrow don the problem.

Read-Only
Author
almak sreem
Posted
6-Jul-2007 08:29 GMT
Toolset
C251
New! Sorted :)

Wohooo

Ive found the problem.

It was the function fat_traverse_entries that was starting at the wrong point.

No simulator used, just an understanding of the code required ;)

Anyone want some working FAT code?

Read-Only
Author
David Rose
Posted
6-Jul-2007 09:14 GMT
Toolset
C251
New! RE: Sorted :)

Almak,

Not sure how you expected anyone to help you find the problem in fat_traverse_entries since you didn't include it in your listings.

But anyway, I guess congratulations are in order.

Read-Only
Author
Dan Henry
Posted
6-Jul-2007 12:48 GMT
Toolset
C251
New! RE: Sorted :)

"Not sure how you expected anyone to help you find the problem in fat_traverse_entries since you didn't include it in your listings."

Based on that, I'd say that instead of us giving him congratulations, he give us an apology. What a waste of time!

Next Thread | Thread List | Previous Thread Start a Thread | Settings