[U-Boot-Users] Bug in fat_register_device() ?

Hello,
I have on a CF Card a MBR with "FAT" on offset 0x36 (sektor 0). The actual U-Boot Code assumes know, that this is a PBR, but it is a MBR ... So I prefer First to check, if it is a valid MBR, and if not, check if it is maybe a PBR. Like the following Patch:
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index a823b5a..d0e99ec 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -83,23 +83,25 @@ fat_register_device(block_dev_desc_t *dev_desc, int part_no) /* no signature found */ return -1; } - if(!strncmp((char *)&buffer[DOS_FS_TYPE_OFFSET],"FAT",3)) { - /* ok, we assume we are on a PBR only */ - cur_part = 1; - part_offset=0; - } - else { #if (CONFIG_COMMANDS & CFG_CMD_IDE) || (CONFIG_COMMANDS & CFG_CMD_SCSI) || \ (CONFIG_COMMANDS & CFG_CMD_USB) || defined(CONFIG_SYSTEMACE) + { disk_partition_t info; if(!get_partition_info(dev_desc, part_no, &info)) { part_offset = info.start; cur_part = part_no; } else { - printf ("** Partition %d not valid on device %d **\n",part_no,dev_desc->dev); - return -1; + if(!strncmp((char *)&buffer[DOS_FS_TYPE_OFFSET],"FAT",3)) { + /* ok, we assume we are on a PBR only */ + cur_part = 1; + part_offset=0; + } else { + printf ("** Partition %d not valid on device %d **\n",part_no,dev_desc->dev); + return -1; + } } + } #else /* FIXME we need to determine the start block of the * partition where the DOS FS resides. This can be done @@ -109,7 +111,6 @@ fat_register_device(block_dev_desc_t *dev_desc, int part_no) part_offset=32; cur_part = 1; #endif - } return 0; }
Some comments? Other ways to distinguish between a MBR or PBR?
Best regards Heiko Schocher -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Dear Heiko,
in message 1180970321.46642d51d24a1@webmail.mnet-online.de you wrote:
#if (CONFIG_COMMANDS & CFG_CMD_IDE) || (CONFIG_COMMANDS & CFG_CMD_SCSI) || \ (CONFIG_COMMANDS & CFG_CMD_USB) || defined(CONFIG_SYSTEMACE)
- {
Incorrect brace style.
disk_partition_t info; if(!get_partition_info(dev_desc, part_no, &info)) { part_offset = info.start; cur_part = part_no; } else {
Ditto (not by you, but lease clean up anyway).
}
- }
#else /* FIXME we need to determine the start block of the
--------^
* partition where the DOS FS resides. This can be done
--------^
@@ -109,7 +111,6 @@ fat_register_device(block_dev_desc_t *dev_desc, int part_no) part_offset=32;
--------^
cur_part = 1;
--------^
Seems you should fix indentation in these lines, and everything inbetween.
Best regards,
Wolfgang Denk

Hello Wolfgang,
Wolfgang Denk wd@denx.de wrote:
#if (CONFIG_COMMANDS & CFG_CMD_IDE) || (CONFIG_COMMANDS & CFG_CMD_SCSI) ||
\
(CONFIG_COMMANDS & CFG_CMD_USB) || defined(CONFIG_SYSTEMACE)
- {
Incorrect brace style.
disk_partition_t info; if(!get_partition_info(dev_desc, part_no, &info)) { part_offset = info.start; cur_part = part_no; } else {
Ditto (not by you, but lease clean up anyway).
}
- }
#else /* FIXME we need to determine the start block of the
--------^
* partition where the DOS FS resides. This can be done
--------^
@@ -109,7 +111,6 @@ fat_register_device(block_dev_desc_t *dev_desc, int
part_no)
part_offset=32;
--------^
cur_part = 1;
--------^
Seems you should fix indentation in these lines, and everything inbetween.
Sorry, didnt had a look at this (was happy to find this issue ;-) Here the new version of the patch:
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index a823b5a..2eaa42b 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -69,10 +69,14 @@ int fat_register_device(block_dev_desc_t *dev_desc, int part_no) { unsigned char buffer[SECTOR_SIZE]; +#if (CONFIG_COMMANDS & CFG_CMD_IDE) || (CONFIG_COMMANDS & CFG_CMD_SCSI) || \ + (CONFIG_COMMANDS & CFG_CMD_USB) || defined(CONFIG_SYSTEMACE) + disk_partition_t info; +#endif
if (!dev_desc->block_read) return -1; - cur_dev=dev_desc; + cur_dev = dev_desc; /* check if we have a MBR (on floppies we have only a PBR) */ if (dev_desc->block_read (dev_desc->dev, 0, 1, (ulong *) buffer) != 1) { printf ("** Can't read from device %d **\n", dev_desc->dev); @@ -83,33 +87,31 @@ fat_register_device(block_dev_desc_t *dev_desc, int part_no) /* no signature found */ return -1; } - if(!strncmp((char *)&buffer[DOS_FS_TYPE_OFFSET],"FAT",3)) { - /* ok, we assume we are on a PBR only */ - cur_part = 1; - part_offset=0; - } - else { #if (CONFIG_COMMANDS & CFG_CMD_IDE) || (CONFIG_COMMANDS & CFG_CMD_SCSI) || \ (CONFIG_COMMANDS & CFG_CMD_USB) || defined(CONFIG_SYSTEMACE) - disk_partition_t info; - if(!get_partition_info(dev_desc, part_no, &info)) { - part_offset = info.start; - cur_part = part_no; - } - else { - printf ("** Partition %d not valid on device %d **\n",part_no,dev_desc->dev); + if(!get_partition_info (dev_desc, part_no, &info)) { + part_offset = info.start; + cur_part = part_no; + } else { + if(!strncmp((char *)&buffer[DOS_FS_TYPE_OFFSET], "FAT", 3)) { + /* ok, we assume we are on a PBR only */ + cur_part = 1; + part_offset = 0; + } else { + printf ("** Partition %d not valid on device %d **\n", + part_no, dev_desc->dev); return -1; } + } #else - /* FIXME we need to determine the start block of the - * partition where the DOS FS resides. This can be done - * by using the get_partition_info routine. For this - * purpose the libpart must be included. - */ - part_offset=32; - cur_part = 1; + /* FIXME we need to determine the start block of the + * partition where the DOS FS resides. This can be done + * by using the get_partition_info routine. For this + * purpose the libpart must be included. + */ + part_offset = 32; + cur_part = 1; #endif - } return 0; }
thanks Heiko

Hi Heiko,
On Tuesday 05 June 2007, hs@denx.de wrote:
- if(!get_partition_info (dev_desc, part_no, &info)) {
Space after the "if" please.
part_offset = info.start;
cur_part = part_no;
- } else {
if(!strncmp((char *)&buffer[DOS_FS_TYPE_OFFSET], "FAT", 3)) {
Here too.
Viele Grüße, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
participants (3)
-
hs@denx.de
-
Stefan Roese
-
Wolfgang Denk