Skip to contentSkip to conversation

FAT: split block device interactions from filesystem logic

PATCH New Pavel Herrmann 2 participants 3 messages Sep 19, 2012 – Sep 23, 2012 VFAT/FAT/msdos filesystem

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 FAT: split block device interactions from filesystem logic

Put block device interaction code into separate file from filesystem logic. This makes it easier to change block device API, and is similar to what other filesystems do.
Cleanup some logic inconsistencies as well.

---
 fs/fat/Makefile    |   4 +-
 fs/fat/dev.c       | 191 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/fat/fat.c       | 175 ++++++------------------------------------------
 fs/fat/fat_write.c |  21 +++---
 include/fat.h      |   7 ++
 5 files changed, 228 insertions(+), 170 deletions(-)
 create mode 100644 fs/fat/dev.c
fs/fat/Makefile +2 -2
index 9635d36..5d10b24 100644
@@ -24,8 +24,8 @@ include $(TOPDIR)/config.mk
 LIB	= $(obj)libfat.o
 
 AOBJS	=
-COBJS-$(CONFIG_CMD_FAT)	:= fat.o
-COBJS-$(CONFIG_FAT_WRITE):= fat_write.o
+COBJS-$(CONFIG_CMD_FAT)	:= fat.o dev.o
+COBJS-$(CONFIG_FAT_WRITE) := fat_write.o dev.o
 
 ifndef CONFIG_SPL_BUILD
 COBJS-$(CONFIG_CMD_FAT)	+= file.o
fs/fat/dev.c new file +191 -0

191 lines not on this page · raw message

fs/fat/fat.c +20 -155

253 lines not on this page · raw message

fs/fat/fat_write.c +8 -13
index a6181e7..2e5bda9 100644
@@ -43,17 +43,12 @@ static void uppercase(char *str, int len)
 static int total_sector;
 static int disk_write(__u32 block, __u32 nr_blocks, void *buf)
 {
-	if (!cur_dev || !cur_dev->block_write)
-		return -1;
-
-	if (cur_part_info.start + block + nr_blocks >
-		cur_part_info.start + total_sector) {
+	if (block + nr_blocks > total_sector) {
 		printf("error: overflow occurs\n");
 		return -1;
 	}
 
-	return cur_dev->block_write(cur_dev->dev,
-			cur_part_info.start + block, nr_blocks,	buf);
+	return fat_disk_write(block, nr_blocks, buf);
 }
 
 /*
@@ -196,7 +191,7 @@ static __u32 get_fatent_value(fsdata *mydata, __u32 entry)
 				return -1;
 		}
 
-		if (disk_read(startblock, getsize, bufptr) < 0) {
+		if (fat_disk_read(startblock, getsize, bufptr) < 0) {
 			debug("Error reading FAT blocks\n");
 			return ret;
 		}
@@ -509,7 +504,7 @@ static int set_fatent_value(fsdata *mydata, __u32 entry, __u32 entry_value)
 				return -1;
 		}
 
-		if (disk_read(startblock, getsize, bufptr) < 0) {
+		if (fat_disk_read(startblock, getsize, bufptr) < 0) {
 			debug("Error reading FAT blocks\n");
 			return -1;
 		}
@@ -793,7 +788,7 @@ static int check_overflow(fsdata *mydata, __u32 clustnum, unsigned long size)
 	if (size % mydata->sect_size)
 		sect_num++;
 
-	if (startsect + sect_num > cur_part_info.start + total_sector)
+	if (startsect + sect_num > total_sector)
 		return -1;
 
 	return 0;
@@ -932,14 +927,14 @@ static int do_fat_write(const char *filename, void *buffer,
 
 	dir_curclust = 0;
 
-	if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
+	if (fat_read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
 		debug("error: reading boot sector\n");
 		return -1;
 	}
 
 	total_sector = bs.total_sect;
 	if (total_sector == 0)
-		total_sector = cur_part_info.size;
+		total_sector = fat_get_partsize();
 
 	if (mydata->fatsize == 32)
 		mydata->fatlength = bs.fat32_length;
@@ -977,7 +972,7 @@ static int do_fat_write(const char *filename, void *buffer,
 		return -1;
 	}
 
-	if (disk_read(cursect,
+	if (fat_disk_read(cursect,
 		(mydata->fatsize == 32) ?
 		(mydata->clust_size) :
 		PREFETCH_BLOCKS, do_fat_read_block) < 0) {
include/fat.h +7 -0
index f1b4a0d..98f77a5 100644
@@ -213,4 +213,11 @@ const char *file_getfsname(int idx);
 int fat_register_device(block_dev_desc_t *dev_desc, int part_no);
 
 int file_fat_write(const char *filename, void *buffer, unsigned long maxsize);
+
+/*internal functions for fs/blockdev API abstraction*/
+int fat_disk_read(__u32 block, __u32 nr_blocks, void *buf);
+int fat_disk_write(__u32 block, __u32 nr_blocks, void *buf);
+int fat_read_bootsectandvi(boot_sector *bs, volume_info *vi, int *fatsize);
+int fat_get_blksz(void);
+int fat_get_partsize(void);
 #endif /* _FAT_H_ */
Signature · 1 line
1.7.12
#1 lore raw

PATCH 1/1 FAT: split block device interactions from filesystem logic

add some CCs

Quoted 591 lines · On Wednesday 19 September 2012 16:34:02 Pavel Herrmann wrote:

Put block device interaction code into separate file from filesystem logic. This makes it easier to change block device API, and is similar to what other filesystems do.
Cleanup some logic inconsistencies as well.

---
 fs/fat/Makefile    |   4 +-
 fs/fat/dev.c       | 191
+++++++++++++++++++++++++++++++++++++++++++++++++++++ fs/fat/fat.c       |
175 ++++++------------------------------------------ fs/fat/fat_write.c | 
21 +++---
 include/fat.h      |   7 ++
 5 files changed, 228 insertions(+), 170 deletions(-)
 create mode 100644 fs/fat/dev.c
fs/fat/Makefile +2 -2
index 9635d36..5d10b24 100644
@@ -24,8 +24,8 @@ include $(TOPDIR)/config.mk
 LIB	= $(obj)libfat.o
 
 AOBJS	=
-COBJS-$(CONFIG_CMD_FAT)	:= fat.o
-COBJS-$(CONFIG_FAT_WRITE):= fat_write.o
+COBJS-$(CONFIG_CMD_FAT)	:= fat.o dev.o
+COBJS-$(CONFIG_FAT_WRITE) := fat_write.o dev.o
 
 ifndef CONFIG_SPL_BUILD
 COBJS-$(CONFIG_CMD_FAT)	+= file.o
fs/fat/dev.c new file +191 -0
new file mode 100644
index 0000000..d5ff0c5
@@ -0,0 +1,191 @@
+/*
+ * (C) Copyright 2012
+ * Pavel Herrmann <[email protected]>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <fat.h>
+#include <part.h>
+#include <errno.h>
+
+#define DOS_BOOT_MAGIC_OFFSET   0x1fe
+#define DOS_FS_TYPE_OFFSET      0x36
+#define DOS_FS32_TYPE_OFFSET    0x52
+
+static block_dev_desc_t *cur_dev;
+static unsigned int cur_part_nr;
+static disk_partition_t cur_part_info;
+
+int fat_disk_read(__u32 block, __u32 nr_blocks, void *buf)
+{
+	if (!cur_dev || !cur_dev->block_read)
+		return -1;
+
+	return cur_dev->block_read(cur_dev->dev,
+		cur_part_info.start + block, nr_blocks, buf);
+}
+
+int fat_disk_write(__u32 block, __u32 nr_blocks, void *buf)
+{
+	if (!cur_dev || !cur_dev->block_read)
+		return -1;
+
+	return cur_dev->block_read(cur_dev->dev,
+		cur_part_info.start + block, nr_blocks, buf);
+}
+
+int fat_get_blksz(void)
+{
+	if (cur_dev == NULL)
+		return -EINVAL;
+	return cur_dev->blksz;
+}
+
+int fat_get_partsize(void)
+{
+	if (cur_dev == NULL)
+		return -EINVAL;
+	return cur_part_info.size;
+}
+
+int fat_register_device(block_dev_desc_t *dev_desc, int part_no)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
+
+	/* First close any currently found FAT filesystem */
+	cur_dev = NULL;
+
+#if (defined(CONFIG_CMD_IDE) || \
+	defined(CONFIG_CMD_SATA) || \
+	defined(CONFIG_CMD_SCSI) || \
+	defined(CONFIG_CMD_USB) || \
+	defined(CONFIG_MMC) || \
+	defined(CONFIG_SYSTEMACE))
+
+	/* Read the partition table, if present */
+	if (!get_partition_info(dev_desc, part_no, &cur_part_info)) {
+		cur_dev = dev_desc;
+		cur_part_nr = part_no;
+	}
+#endif
+
+	/* Otherwise it might be a superfloppy (whole-disk FAT filesystem) */
+	if (!cur_dev) {
+		if (part_no != 1) {
+			printf("** Partition %d not valid on device %d **\n",
+			part_no, dev_desc->dev);
+			return -1;
+		}
+
+		cur_dev = dev_desc;
+		cur_part_nr = 1;
+		cur_part_info.start = 0;
+		cur_part_info.size = dev_desc->lba;
+		cur_part_info.blksz = dev_desc->blksz;
+		memset(cur_part_info.name, 0, sizeof(cur_part_info.name));
+		memset(cur_part_info.type, 0, sizeof(cur_part_info.type));
+	}
+
+	/* Make sure it has a valid FAT header */
+	if (fat_disk_read(0, 1, buffer) != 1) {
+		cur_dev = NULL;
+		return -1;
+	}
+
+	/* Check if it's actually a DOS volume */
+	if (memcmp(buffer + DOS_BOOT_MAGIC_OFFSET, "\x55\xAA", 2)) {
+		cur_dev = NULL;
+		return -1;
+	}
+
+	/* Check for FAT12/FAT16/FAT32 filesystem */
+	if (!memcmp(buffer + DOS_FS_TYPE_OFFSET, "FAT", 3))
+		return 0;
+	if (!memcmp(buffer + DOS_FS32_TYPE_OFFSET, "FAT32", 5))
+		return 0;
+
+	cur_dev = NULL;
+	return -1;
+}
+
+int file_fat_detectfs(void)
+{
+	boot_sector bs;
+	volume_info volinfo;
+	int fatsize;
+	char vol_label[12];
+
+	if (cur_dev == NULL) {
+		printf("No current device\n");
+		return 1;
+	}
+
+#if defined(CONFIG_CMD_IDE) || \
+	defined(CONFIG_CMD_SATA) || \
+	defined(CONFIG_CMD_SCSI) || \
+	defined(CONFIG_CMD_USB) || \
+	defined(CONFIG_MMC)
+	printf("Interface:  ");
+	switch (cur_dev->if_type) {
+	case IF_TYPE_IDE:
+		printf("IDE");
+		break;
+	case IF_TYPE_SATA:
+		printf("SATA");
+		break;
+	case IF_TYPE_SCSI:
+		printf("SCSI");
+		break;
+	case IF_TYPE_ATAPI:
+		printf("ATAPI");
+		break;
+	case IF_TYPE_USB:
+		printf("USB");
+		break;
+	case IF_TYPE_DOC:
+		printf("DOC");
+		break;
+	case IF_TYPE_MMC:
+		printf("MMC");
+		break;
+	default:
+		printf("Unknown");
+	}
+
+	printf("\n  Device %d: ", cur_dev->dev);
+	dev_print(cur_dev);
+#endif
+
+	if (fat_read_bootsectandvi(&bs, &volinfo, &fatsize)) {
+		printf("\nNo valid FAT fs found\n");
+		return 1;
+	}
+
+	memcpy(vol_label, volinfo.volume_label, 11);
+	vol_label[11] = '\0';
+	volinfo.fs_type[5] = '\0';
+
+	printf("Partition %d: Filesystem: %s \"%s\"\n", cur_part_nr,
+		volinfo.fs_type, vol_label);
+
+	return 0;
+}
+
fs/fat/fat.c +0 -78
index 19f6a8c..a13c62b 100644
@@ -45,84 +45,6 @@ static void downcase(char *str)
 	}
 }
 
-static block_dev_desc_t *cur_dev;
-static unsigned int cur_part_nr;
-static disk_partition_t cur_part_info;
-
-#define DOS_BOOT_MAGIC_OFFSET	0x1fe
-#define DOS_FS_TYPE_OFFSET	0x36
-#define DOS_FS32_TYPE_OFFSET	0x52
-
-static int disk_read(__u32 block, __u32 nr_blocks, void *buf)
-{
-	if (!cur_dev || !cur_dev->block_read)
-		return -1;
-
-	return cur_dev->block_read(cur_dev->dev,
-			cur_part_info.start + block, nr_blocks, buf);
-}
-
-int fat_register_device(block_dev_desc_t * dev_desc, int part_no)
-{
-	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
-
-	/* First close any currently found FAT filesystem */
-	cur_dev = NULL;
-
-#if (defined(CONFIG_CMD_IDE) || \
-     defined(CONFIG_CMD_SATA) || \
-     defined(CONFIG_CMD_SCSI) || \
-     defined(CONFIG_CMD_USB) || \
-     defined(CONFIG_MMC) || \
-     defined(CONFIG_SYSTEMACE) )
-
-	/* Read the partition table, if present */
-	if (!get_partition_info(dev_desc, part_no, &cur_part_info)) {
-		cur_dev = dev_desc;
-		cur_part_nr = part_no;
-	}
-#endif
-
-	/* Otherwise it might be a superfloppy (whole-disk FAT filesystem) */
-	if (!cur_dev) {
-		if (part_no != 1) {
-			printf("** Partition %d not valid on device %d **\n",
-					part_no, dev_desc->dev);
-			return -1;
-		}
-
-		cur_dev = dev_desc;
-		cur_part_nr = 1;
-		cur_part_info.start = 0;
-		cur_part_info.size = dev_desc->lba;
-		cur_part_info.blksz = dev_desc->blksz;
-		memset(cur_part_info.name, 0, sizeof(cur_part_info.name));
-		memset(cur_part_info.type, 0, sizeof(cur_part_info.type));
-	}
-
-	/* Make sure it has a valid FAT header */
-	if (disk_read(0, 1, buffer) != 1) {
-		cur_dev = NULL;
-		return -1;
-	}
-
-	/* Check if it's actually a DOS volume */
-	if (memcmp(buffer + DOS_BOOT_MAGIC_OFFSET, "\x55\xAA", 2)) {
-		cur_dev = NULL;
-		return -1;
-	}
-
-	/* Check for FAT12/FAT16/FAT32 filesystem */
-	if (!memcmp(buffer + DOS_FS_TYPE_OFFSET, "FAT", 3))
-		return 0;
-	if (!memcmp(buffer + DOS_FS32_TYPE_OFFSET, "FAT32", 5))
-		return 0;
-
-	cur_dev = NULL;
-	return -1;
-}
-
-
 /*
  * Get the first occurence of a directory delimiter ('/' or '\') in a

string. * Return index into string if found, -1 otherwise.

fs/fat/fat.c +2 -2
@@ -212,7 +134,7 @@ static __u32 get_fatent(fsdata *mydata, __u32 entry)
 
 		startblock += mydata->fat_sect;	/* Offset from start of disk */
 
-		if (disk_read(startblock, getsize, bufptr) < 0) {
+		if (fat_disk_read(startblock, getsize, bufptr) < 0) {
 			debug("Error reading FAT blocks\n");
 			return ret;
 		}
@@ -290,7 +212,7 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8
 *buffer, unsigned long size) printf("FAT: Misaligned buffer address
 (%p)\n", buffer);
 
 		while (size >= mydata->sect_size) {
-			ret = disk_read(startsect++, 1, tmpbuf);
+			ret = fat_disk_read(startsect++, 1, tmpbuf);
 			if (ret != 1) {
 				debug("Error reading data (got %d)\n", ret);
fs/fat/fat.c +2 -2
 				return -1;
@@ -302,7 +224,7 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8
 *buffer, unsigned long size) }
 	} else {
 		idx = size / mydata->sect_size;
-		ret = disk_read(startsect, idx, buffer);
+		ret = fat_disk_read(startsect, idx, buffer);
 		if (ret != idx) {
 			debug("Error reading data (got %d)\n", ret);
 			return -1;
@@ -315,7 +237,7 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8
 *buffer, unsigned long size) if (size) {
 		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
 
-		ret = disk_read(startsect, 1, tmpbuf);
+		ret = fat_disk_read(startsect, 1, tmpbuf);
 		if (ret != 1) {
 			debug("Error reading data (got %d)\n", ret);
 			return -1;
@@ -702,25 +624,28 @@ static dir_entry *get_dentfromdir(fsdata *mydata, int

startsect, /*
* Read boot sector and volume info from a FAT filesystem */

fs/fat/fat.c +2 -2
-static int
-read_bootsectandvi(boot_sector *bs, volume_info *volinfo, int *fatsize)
+int
+fat_read_bootsectandvi(boot_sector *bs, volume_info *volinfo, int *fatsize)

{

fs/fat/fat.c +14 -71
 	__u8 *block;
 	volume_info *vistart;
 	int ret = 0;
+	int blksz;
 
-	if (cur_dev == NULL) {
+	blksz = fat_get_blksz();
+	if (blksz <= 0) {
+		/*this means the device is NULL, or otherwise unavailable*/
 		debug("Error: no device selected\n");
 		return -1;
 	}
 
-	block = memalign(ARCH_DMA_MINALIGN, cur_dev->blksz);
+	block = memalign(ARCH_DMA_MINALIGN, blksz);
 	if (block == NULL) {
 		debug("Error: allocating block\n");
 		return -1;
 	}
 
-	if (disk_read(0, 1, block) < 0) {
+	if (fat_disk_read(0, 1, block) < 0) {
 		debug("Error: reading block\n");
 		goto fail;
 	}
@@ -792,11 +717,14 @@ do_fat_read(const char *filename, void *buffer,
 unsigned long maxsize, int dols) __u32 root_cluster = 0;
 	int rootdir_size = 0;
 	int j;
+	int blksz;
 
-	if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
+	if (fat_read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
 		debug("Error: reading boot sector\n");
 		return -1;
 	}
+	/*this will always succeed, because it did in above call*/
+	blksz = fat_get_blksz();
 
 	if (mydata->fatsize == 32) {
 		root_cluster = bs.root_cluster;
@@ -812,9 +740,9 @@ do_fat_read(const char *filename, void *buffer, unsigned
 long maxsize, int dols)
 
 	mydata->sect_size = (bs.sector_size[1] << 8) + bs.sector_size[0];
 	mydata->clust_size = bs.cluster_size;
-	if (mydata->sect_size != cur_part_info.blksz) {
-		printf("Error: FAT sector size mismatch (fs=%hu, dev=%lu)\n",
-				mydata->sect_size, cur_part_info.blksz);
+	if (mydata->sect_size != blksz) {
+		printf("Error: FAT sector size mismatch (fs=%hu, dev=%d)\n",
+				mydata->sect_size, blksz);
 		return -1;
 	}
@@ -884,7 +812,7 @@ do_fat_read(const char *filename, void *buffer, unsigned
 long maxsize, int dols) debug("FAT read sect=%d, clust_size=%d,
 DIRENTSPERBLOCK=%zd\n", cursect, mydata->clust_size, DIRENTSPERBLOCK);
 
-			if (disk_read(cursect,
+			if (fat_disk_read(cursect,
 					(mydata->fatsize == 32) ?
 					(mydata->clust_size) :
 					PREFETCH_BLOCKS,
@@ -1124,69 +1052,6 @@ exit:
 	return ret;
 }
 
-int file_fat_detectfs(void)
-{
-	boot_sector bs;
-	volume_info volinfo;
-	int fatsize;
-	char vol_label[12];
-
-	if (cur_dev == NULL) {
-		printf("No current device\n");
-		return 1;
-	}
-
-#if defined(CONFIG_CMD_IDE) || \
-    defined(CONFIG_CMD_SATA) || \
-    defined(CONFIG_CMD_SCSI) || \
-    defined(CONFIG_CMD_USB) || \
-    defined(CONFIG_MMC)
-	printf("Interface:  ");
-	switch (cur_dev->if_type) {
-	case IF_TYPE_IDE:
-		printf("IDE");
-		break;
-	case IF_TYPE_SATA:
-		printf("SATA");
-		break;
-	case IF_TYPE_SCSI:
-		printf("SCSI");
-		break;
-	case IF_TYPE_ATAPI:
-		printf("ATAPI");
-		break;
-	case IF_TYPE_USB:
-		printf("USB");
-		break;
-	case IF_TYPE_DOC:
-		printf("DOC");
-		break;
-	case IF_TYPE_MMC:
-		printf("MMC");
-		break;
-	default:
-		printf("Unknown");
-	}
-
-	printf("\n  Device %d: ", cur_dev->dev);
-	dev_print(cur_dev);
-#endif
-
-	if (read_bootsectandvi(&bs, &volinfo, &fatsize)) {
-		printf("\nNo valid FAT fs found\n");
-		return 1;
-	}
-
-	memcpy(vol_label, volinfo.volume_label, 11);
-	vol_label[11] = '\0';
-	volinfo.fs_type[5] = '\0';
-
-	printf("Partition %d: Filesystem: %s \"%s\"\n", cur_part_nr,
-		volinfo.fs_type, vol_label);
-
-	return 0;
-}
-
 int file_fat_ls(const char *dir)
 {
 	return do_fat_read(dir, NULL, 0, LS_YES);
fs/fat/fat_write.c +7 -12
index a6181e7..2e5bda9 100644
@@ -43,17 +43,12 @@ static void uppercase(char *str, int len)
 static int total_sector;
 static int disk_write(__u32 block, __u32 nr_blocks, void *buf)
 {
-	if (!cur_dev || !cur_dev->block_write)
-		return -1;
-
-	if (cur_part_info.start + block + nr_blocks >
-		cur_part_info.start + total_sector) {
+	if (block + nr_blocks > total_sector) {
 		printf("error: overflow occurs\n");
 		return -1;
 	}
 
-	return cur_dev->block_write(cur_dev->dev,
-			cur_part_info.start + block, nr_blocks,	buf);
+	return fat_disk_write(block, nr_blocks, buf);
 }
 
 /*
@@ -196,7 +191,7 @@ static __u32 get_fatent_value(fsdata *mydata, __u32
 entry) return -1;
 		}
 
-		if (disk_read(startblock, getsize, bufptr) < 0) {
+		if (fat_disk_read(startblock, getsize, bufptr) < 0) {
 			debug("Error reading FAT blocks\n");
 			return ret;
 		}
@@ -509,7 +504,7 @@ static int set_fatent_value(fsdata *mydata, __u32 entry,
 __u32 entry_value) return -1;
 		}
 
-		if (disk_read(startblock, getsize, bufptr) < 0) {
+		if (fat_disk_read(startblock, getsize, bufptr) < 0) {
 			debug("Error reading FAT blocks\n");
 			return -1;
 		}
@@ -793,7 +788,7 @@ static int check_overflow(fsdata *mydata, __u32
 clustnum, unsigned long size) if (size % mydata->sect_size)
 		sect_num++;
 
-	if (startsect + sect_num > cur_part_info.start + total_sector)
+	if (startsect + sect_num > total_sector)
 		return -1;
 
 	return 0;
@@ -932,14 +927,14 @@ static int do_fat_write(const char *filename, void
 *buffer,
 
 	dir_curclust = 0;
 
-	if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
+	if (fat_read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
 		debug("error: reading boot sector\n");
 		return -1;
 	}
 
 	total_sector = bs.total_sect;
 	if (total_sector == 0)
-		total_sector = cur_part_info.size;
+		total_sector = fat_get_partsize();
 
 	if (mydata->fatsize == 32)
fs/fat/fat_write.c +1 -1
 		mydata->fatlength = bs.fat32_length;
@@ -977,7 +972,7 @@ static int do_fat_write(const char *filename, void
 *buffer, return -1;
 	}
 
-	if (disk_read(cursect,
+	if (fat_disk_read(cursect,
 		(mydata->fatsize == 32) ?
 		(mydata->clust_size) :
 		PREFETCH_BLOCKS, do_fat_read_block) < 0) {
include/fat.h +0 -0
index f1b4a0d..98f77a5 100644
@@ -213,4 +213,11 @@ const char *file_getfsname(int idx);
 int fat_register_device(block_dev_desc_t *dev_desc, int part_no);
 
 int file_fat_write(const char *filename, void *buffer, unsigned long

maxsize); +

include/fat.h +6 -0
+/*internal functions for fs/blockdev API abstraction*/
+int fat_disk_read(__u32 block, __u32 nr_blocks, void *buf);
+int fat_disk_write(__u32 block, __u32 nr_blocks, void *buf);
+int fat_read_bootsectandvi(boot_sector *bs, volume_info *vi, int *fatsize);
+int fat_get_blksz(void);
+int fat_get_partsize(void);
 #endif /* _FAT_H_ */
#2 lore raw

PATCH 1/1 FAT: split block device interactions from filesystem logic

Hi Pavel,

Quoted 9 lines · Pavel Herrmann

add some CCs

Quoted 6 lines · On Wednesday 19 September 2012 16:34:02 Pavel Herrmann wrote:

Put block device interaction code into separate file from filesystem logic.
This makes it easier to change block device API, and is similar to what
other filesystems do.
Cleanup some logic inconsistencies as well.

Sounds good. See my comments below.

This might need some light rework to make it applicable after
http://patchwork.ozlabs.org/patch/184793/, which should be applied soon.

Quoted 26 lines
Quoted 26 lines
---
 fs/fat/Makefile    |   4 +-
 fs/fat/dev.c       | 191
+++++++++++++++++++++++++++++++++++++++++++++++++++++ fs/fat/fat.c
      |
175 ++++++------------------------------------------
fs/fat/fat_write.c |
21 +++---
 include/fat.h      |   7 ++
 5 files changed, 228 insertions(+), 170 deletions(-)
 create mode 100644 fs/fat/dev.c
fs/fat/Makefile +2 -2
index 9635d36..5d10b24 100644
@@ -24,8 +24,8 @@ include $(TOPDIR)/config.mk
 LIB	= $(obj)libfat.o
 
 AOBJS	=
-COBJS-$(CONFIG_CMD_FAT)	:= fat.o
-COBJS-$(CONFIG_FAT_WRITE):= fat_write.o
+COBJS-$(CONFIG_CMD_FAT)	:= fat.o dev.o
+COBJS-$(CONFIG_FAT_WRITE) := fat_write.o dev.o
 
Quoted 11 lines

fs/fat/dev.c line 3

Quoted 11 lines
fs/fat/Makefile +0 -0
 ifndef CONFIG_SPL_BUILD
 COBJS-$(CONFIG_CMD_FAT)	+= file.o
fs/fat/dev.c new file +3 -0
new file mode 100644
index 0000000..d5ff0c5
@@ -0,0 +1,191 @@
+/*
+ * (C) Copyright 2012
+ * Pavel Herrmann <[email protected]>

Since 99% of the code here comes from fat.c and fat_write.c, the copyright information from these files should probably be added here too.

Quoted 43 lines
Quoted 43 lines
fs/fat/dev.c new file +15 -0
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public

License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */

fs/fat/dev.c new file +23 -0
+
+#include <common.h>
+#include <fat.h>
+#include <part.h>
+#include <errno.h>
+
+#define DOS_BOOT_MAGIC_OFFSET   0x1fe
+#define DOS_FS_TYPE_OFFSET      0x36
+#define DOS_FS32_TYPE_OFFSET    0x52
+
+static block_dev_desc_t *cur_dev;
+static unsigned int cur_part_nr;
+static disk_partition_t cur_part_info;
+
+int fat_disk_read(__u32 block, __u32 nr_blocks, void *buf)
+{
+	if (!cur_dev || !cur_dev->block_read)
+		return -1;
+
+	return cur_dev->block_read(cur_dev->dev,
+		cur_part_info.start + block, nr_blocks, buf);
+}
+
Quoted 8 lines

fs/fat/dev.c line 53

Quoted 8 lines
fs/fat/dev.c new file +8 -0
+int fat_disk_write(__u32 block, __u32 nr_blocks, void *buf)
+{
+	if (!cur_dev || !cur_dev->block_read)
+		return -1;
+
+	return cur_dev->block_read(cur_dev->dev,
+		cur_part_info.start + block, nr_blocks, buf);
+}

block_write...

Please, split this patch into:
- moved code + renaming,
- behavior changes (one patch per behavior change).
That will make things clearer and easier to review.

Quoted 2 lines

fs/fat/dev.c

Quoted 2 lines
fs/fat/dev.c new file +2 -0
+
+int fat_get_blksz(void)

cur_dev->blksz is ulong, so long might be better here, even if it does not seem strictly necessary.

Quoted 4 lines

fs/fat/dev.c line 59

Quoted 4 lines
fs/fat/dev.c new file +4 -0
+{
+	if (cur_dev == NULL)
+		return -EINVAL;
+	return cur_dev->blksz;

Or cur_part_info.blksize? Your calls to this function replace both usages, so please make sure that this won't cause any trouble.

Quoted 3 lines

fs/fat/dev.c line 62

Quoted 3 lines
fs/fat/dev.c new file +3 -0
+}
+
+int fat_get_partsize(void)

long?

Quoted 3 lines

fs/fat/dev.c line 58

Quoted 3 lines
fs/fat/dev.c new file +3 -0
+{
+	if (cur_dev == NULL)
+		return -EINVAL;

This test looks unrelated to the line below and changes the behavior where this function is called. Please clarify.

Quoted 317 lines
Quoted 317 lines
fs/fat/dev.c new file +25 -0
+	return cur_part_info.size;
+}
+
+int fat_register_device(block_dev_desc_t *dev_desc, int part_no)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
+
+	/* First close any currently found FAT filesystem */
+	cur_dev = NULL;
+
+#if (defined(CONFIG_CMD_IDE) || \
+	defined(CONFIG_CMD_SATA) || \
+	defined(CONFIG_CMD_SCSI) || \
+	defined(CONFIG_CMD_USB) || \
+	defined(CONFIG_MMC) || \
+	defined(CONFIG_SYSTEMACE))
+
+	/* Read the partition table, if present */
+	if (!get_partition_info(dev_desc, part_no, &cur_part_info)) {
+		cur_dev = dev_desc;
+		cur_part_nr = part_no;
+	}
+#endif
+
+	/* Otherwise it might be a superfloppy (whole-disk FAT

filesystem) */

fs/fat/dev.c new file +101 -0
+	if (!cur_dev) {
+		if (part_no != 1) {
+			printf("** Partition %d not valid on device %d **\n",
+			part_no, dev_desc->dev);
+			return -1;
+		}
+
+		cur_dev = dev_desc;
+		cur_part_nr = 1;
+		cur_part_info.start = 0;
+		cur_part_info.size = dev_desc->lba;
+		cur_part_info.blksz = dev_desc->blksz;
+		memset(cur_part_info.name, 0, sizeof(cur_part_info.name));
+		memset(cur_part_info.type, 0, sizeof(cur_part_info.type));
+	}
+
+	/* Make sure it has a valid FAT header */
+	if (fat_disk_read(0, 1, buffer) != 1) {
+		cur_dev = NULL;
+		return -1;
+	}
+
+	/* Check if it's actually a DOS volume */
+	if (memcmp(buffer + DOS_BOOT_MAGIC_OFFSET, "\x55\xAA", 2)) {
+		cur_dev = NULL;
+		return -1;
+	}
+
+	/* Check for FAT12/FAT16/FAT32 filesystem */
+	if (!memcmp(buffer + DOS_FS_TYPE_OFFSET, "FAT", 3))
+		return 0;
+	if (!memcmp(buffer + DOS_FS32_TYPE_OFFSET, "FAT32", 5))
+		return 0;
+
+	cur_dev = NULL;
+	return -1;
+}
+
+int file_fat_detectfs(void)
+{
+	boot_sector bs;
+	volume_info volinfo;
+	int fatsize;
+	char vol_label[12];
+
+	if (cur_dev == NULL) {
+		printf("No current device\n");
+		return 1;
+	}
+
+#if defined(CONFIG_CMD_IDE) || \
+	defined(CONFIG_CMD_SATA) || \
+	defined(CONFIG_CMD_SCSI) || \
+	defined(CONFIG_CMD_USB) || \
+	defined(CONFIG_MMC)
+	printf("Interface:  ");
+	switch (cur_dev->if_type) {
+	case IF_TYPE_IDE:
+		printf("IDE");
+		break;
+	case IF_TYPE_SATA:
+		printf("SATA");
+		break;
+	case IF_TYPE_SCSI:
+		printf("SCSI");
+		break;
+	case IF_TYPE_ATAPI:
+		printf("ATAPI");
+		break;
+	case IF_TYPE_USB:
+		printf("USB");
+		break;
+	case IF_TYPE_DOC:
+		printf("DOC");
+		break;
+	case IF_TYPE_MMC:
+		printf("MMC");
+		break;
+	default:
+		printf("Unknown");
+	}
+
+	printf("\n  Device %d: ", cur_dev->dev);
+	dev_print(cur_dev);
+#endif
+
+	if (fat_read_bootsectandvi(&bs, &volinfo, &fatsize)) {
+		printf("\nNo valid FAT fs found\n");
+		return 1;
+	}
+
+	memcpy(vol_label, volinfo.volume_label, 11);
+	vol_label[11] = '\0';
+	volinfo.fs_type[5] = '\0';
+
+	printf("Partition %d: Filesystem: %s \"%s\"\n", cur_part_nr,
+		volinfo.fs_type, vol_label);
+
+	return 0;
+}
+
fs/fat/fat.c +0 -39
index 19f6a8c..a13c62b 100644
@@ -45,84 +45,6 @@ static void downcase(char *str)
 	}
 }
 
-static block_dev_desc_t *cur_dev;
-static unsigned int cur_part_nr;
-static disk_partition_t cur_part_info;
-
-#define DOS_BOOT_MAGIC_OFFSET	0x1fe
-#define DOS_FS_TYPE_OFFSET	0x36
-#define DOS_FS32_TYPE_OFFSET	0x52
-
-static int disk_read(__u32 block, __u32 nr_blocks, void *buf)
-{
-	if (!cur_dev || !cur_dev->block_read)
-		return -1;
-
-	return cur_dev->block_read(cur_dev->dev,
-			cur_part_info.start + block, nr_blocks, buf);
-}
-
-int fat_register_device(block_dev_desc_t * dev_desc, int part_no)
-{
-	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
-
-	/* First close any currently found FAT filesystem */
-	cur_dev = NULL;
-
-#if (defined(CONFIG_CMD_IDE) || \
-     defined(CONFIG_CMD_SATA) || \
-     defined(CONFIG_CMD_SCSI) || \
-     defined(CONFIG_CMD_USB) || \
-     defined(CONFIG_MMC) || \
-     defined(CONFIG_SYSTEMACE) )
-
-	/* Read the partition table, if present */
-	if (!get_partition_info(dev_desc, part_no, &cur_part_info)) {
-		cur_dev = dev_desc;
-		cur_part_nr = part_no;
-	}
-#endif
-
-	/* Otherwise it might be a superfloppy (whole-disk FAT

filesystem) */

fs/fat/fat.c +0 -39
-	if (!cur_dev) {
-		if (part_no != 1) {
-			printf("** Partition %d not valid on device %d **\n",
-					part_no, dev_desc->dev);
-			return -1;
-		}
-
-		cur_dev = dev_desc;
-		cur_part_nr = 1;
-		cur_part_info.start = 0;
-		cur_part_info.size = dev_desc->lba;
-		cur_part_info.blksz = dev_desc->blksz;
-		memset(cur_part_info.name, 0, sizeof(cur_part_info.name));
-		memset(cur_part_info.type, 0, sizeof(cur_part_info.type));
-	}
-
-	/* Make sure it has a valid FAT header */
-	if (disk_read(0, 1, buffer) != 1) {
-		cur_dev = NULL;
-		return -1;
-	}
-
-	/* Check if it's actually a DOS volume */
-	if (memcmp(buffer + DOS_BOOT_MAGIC_OFFSET, "\x55\xAA", 2)) {
-		cur_dev = NULL;
-		return -1;
-	}
-
-	/* Check for FAT12/FAT16/FAT32 filesystem */
-	if (!memcmp(buffer + DOS_FS_TYPE_OFFSET, "FAT", 3))
-		return 0;
-	if (!memcmp(buffer + DOS_FS32_TYPE_OFFSET, "FAT32", 5))
-		return 0;
-
-	cur_dev = NULL;
-	return -1;
-}
-
-
 /*
  * Get the first occurence of a directory delimiter ('/' or '\')
  in a

string. * Return index into string if found, -1 otherwise.

fs/fat/fat.c +1 -1
@@ -212,7 +134,7 @@ static __u32 get_fatent(fsdata *mydata, __u32
 entry)
 
 		startblock += mydata->fat_sect;	/* Offset from start of disk */
 
-		if (disk_read(startblock, getsize, bufptr) < 0) {
+		if (fat_disk_read(startblock, getsize, bufptr) < 0) {
 			debug("Error reading FAT blocks\n");
 			return ret;
fs/fat/fat.c +0 -0
 		}
@@ -290,7 +212,7 @@ get_cluster(fsdata *mydata, __u32 clustnum,

__u8
*buffer, unsigned long size) printf("FAT: Misaligned buffer address (%p)\n", buffer);

fs/fat/fat.c +1 -1
 		while (size >= mydata->sect_size) {
-			ret = disk_read(startsect++, 1, tmpbuf);
+			ret = fat_disk_read(startsect++, 1, tmpbuf);
 			if (ret != 1) {
 				debug("Error reading data (got %d)\n", ret);
 				return -1;
@@ -302,7 +224,7 @@ get_cluster(fsdata *mydata, __u32 clustnum,

__u8
*buffer, unsigned long size) }

fs/fat/fat.c +1 -1
 	} else {
 		idx = size / mydata->sect_size;
-		ret = disk_read(startsect, idx, buffer);
+		ret = fat_disk_read(startsect, idx, buffer);
 		if (ret != idx) {
 			debug("Error reading data (got %d)\n", ret);
 			return -1;
@@ -315,7 +237,7 @@ get_cluster(fsdata *mydata, __u32 clustnum,

__u8
*buffer, unsigned long size) if (size) {

fs/fat/fat.c +1 -1
 		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
 
-		ret = disk_read(startsect, 1, tmpbuf);
+		ret = fat_disk_read(startsect, 1, tmpbuf);
 		if (ret != 1) {
 			debug("Error reading data (got %d)\n", ret);
 			return -1;
@@ -702,25 +624,28 @@ static dir_entry *get_dentfromdir(fsdata
 *mydata, int

startsect, /*
* Read boot sector and volume info from a FAT filesystem */

fs/fat/fat.c +0 -2
-static int
-read_bootsectandvi(boot_sector *bs, volume_info *volinfo, int

*fatsize)

fs/fat/fat.c +2 -0
+int
+fat_read_bootsectandvi(boot_sector *bs, volume_info *volinfo, int

*fatsize)
{

fs/fat/fat.c +10 -4
 	__u8 *block;
 	volume_info *vistart;
 	int ret = 0;
+	int blksz;
 
-	if (cur_dev == NULL) {
+	blksz = fat_get_blksz();
+	if (blksz <= 0) {
+		/*this means the device is NULL, or otherwise unavailable*/
 		debug("Error: no device selected\n");
 		return -1;
 	}
 
-	block = memalign(ARCH_DMA_MINALIGN, cur_dev->blksz);
+	block = memalign(ARCH_DMA_MINALIGN, blksz);
 	if (block == NULL) {
 		debug("Error: allocating block\n");
 		return -1;
 	}
 
-	if (disk_read(0, 1, block) < 0) {
+	if (fat_disk_read(0, 1, block) < 0) {
 		debug("Error: reading block\n");
 		goto fail;
 	}
@@ -792,11 +717,14 @@ do_fat_read(const char *filename, void
 *buffer,
 unsigned long maxsize, int dols) __u32 root_cluster = 0;
 	int rootdir_size = 0;
 	int j;
+	int blksz;
 
-	if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
+	if (fat_read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
 		debug("Error: reading boot sector\n");
 		return -1;
 	}
+	/*this will always succeed, because it did in above call*/
+	blksz = fat_get_blksz();
 
 	if (mydata->fatsize == 32) {
fs/fat/fat.c +0 -0
 		root_cluster = bs.root_cluster;
@@ -812,9 +740,9 @@ do_fat_read(const char *filename, void *buffer,

unsigned
long maxsize, int dols)

Quoted 6 lines

fs/fat/fat.c do_fat_read(const char *filename, void *buffer, unsigned long maxsize, int dols) · line 743

Quoted 6 lines
fs/fat/fat.c +1 -3
 	mydata->sect_size = (bs.sector_size[1] << 8) + bs.sector_size[0];
 	mydata->clust_size = bs.cluster_size;
-	if (mydata->sect_size != cur_part_info.blksz) {
-		printf("Error: FAT sector size mismatch (fs=%hu, dev=%lu)\n",
-				mydata->sect_size, cur_part_info.blksz);
+	if (mydata->sect_size != blksz) {

cur_part_info vs. cur_dev OK?

Quoted 82 lines
Quoted 82 lines
fs/fat/fat.c +2 -0
+		printf("Error: FAT sector size mismatch (fs=%hu, dev=%d)\n",
+				mydata->sect_size, blksz);
 		return -1;
 	}
@@ -884,7 +812,7 @@ do_fat_read(const char *filename, void *buffer,

unsigned
long maxsize, int dols) debug("FAT read sect=%d, clust_size=%d, DIRENTSPERBLOCK=%zd\n", cursect, mydata->clust_size, DIRENTSPERBLOCK);

fs/fat/fat.c +1 -63
-			if (disk_read(cursect,
+			if (fat_disk_read(cursect,
 					(mydata->fatsize == 32) ?
 					(mydata->clust_size) :
 					PREFETCH_BLOCKS,
@@ -1124,69 +1052,6 @@ exit:
 	return ret;
 }
 
-int file_fat_detectfs(void)
-{
-	boot_sector bs;
-	volume_info volinfo;
-	int fatsize;
-	char vol_label[12];
-
-	if (cur_dev == NULL) {
-		printf("No current device\n");
-		return 1;
-	}
-
-#if defined(CONFIG_CMD_IDE) || \
-    defined(CONFIG_CMD_SATA) || \
-    defined(CONFIG_CMD_SCSI) || \
-    defined(CONFIG_CMD_USB) || \
-    defined(CONFIG_MMC)
-	printf("Interface:  ");
-	switch (cur_dev->if_type) {
-	case IF_TYPE_IDE:
-		printf("IDE");
-		break;
-	case IF_TYPE_SATA:
-		printf("SATA");
-		break;
-	case IF_TYPE_SCSI:
-		printf("SCSI");
-		break;
-	case IF_TYPE_ATAPI:
-		printf("ATAPI");
-		break;
-	case IF_TYPE_USB:
-		printf("USB");
-		break;
-	case IF_TYPE_DOC:
-		printf("DOC");
-		break;
-	case IF_TYPE_MMC:
-		printf("MMC");
-		break;
-	default:
-		printf("Unknown");
-	}
-
-	printf("\n  Device %d: ", cur_dev->dev);
-	dev_print(cur_dev);
-#endif
-
-	if (read_bootsectandvi(&bs, &volinfo, &fatsize)) {
-		printf("\nNo valid FAT fs found\n");
-		return 1;
-	}
-
-	memcpy(vol_label, volinfo.volume_label, 11);
-	vol_label[11] = '\0';
-	volinfo.fs_type[5] = '\0';
-
-	printf("Partition %d: Filesystem: %s \"%s\"\n", cur_part_nr,
-		volinfo.fs_type, vol_label);
-
-	return 0;
-}
Quoted 8 lines

fs/fat/fat_write.c

Quoted 8 lines
fs/fat/fat.c +0 -1
-
 int file_fat_ls(const char *dir)
 {
 	return do_fat_read(dir, NULL, 0, LS_YES);
fs/fat/fat_write.c +0 -0
index a6181e7..2e5bda9 100644

Please get rid of the '#include "fat.c"'. This is the perfect opportunity to do so.

Quoted 14 lines
Quoted 14 lines
fs/fat/fat_write.c +1 -5
@@ -43,17 +43,12 @@ static void uppercase(char *str, int len)
 static int total_sector;
 static int disk_write(__u32 block, __u32 nr_blocks, void *buf)
 {
-	if (!cur_dev || !cur_dev->block_write)
-		return -1;
-
-	if (cur_part_info.start + block + nr_blocks >
-		cur_part_info.start + total_sector) {
+	if (block + nr_blocks > total_sector) {
 		printf("error: overflow occurs\n");
 		return -1;
 	}
 
Quoted 4 lines

fs/fat/fat_write.c static void uppercase(char *str, int len) · line 52

Quoted 4 lines
fs/fat/fat_write.c +1 -2
-	return cur_dev->block_write(cur_dev->dev,
-			cur_part_info.start + block, nr_blocks,	buf);
+	return fat_disk_write(block, nr_blocks, buf);
 }

Why don't you move that to dev.c as well?

Quoted 26 lines
Quoted 26 lines
fs/fat/fat_write.c +0 -0
 /*
@@ -196,7 +191,7 @@ static __u32 get_fatent_value(fsdata *mydata,

__u32
entry) return -1;

fs/fat/fat_write.c +2 -2
 		}
 
-		if (disk_read(startblock, getsize, bufptr) < 0) {
+		if (fat_disk_read(startblock, getsize, bufptr) < 0) {
 			debug("Error reading FAT blocks\n");
 			return ret;
 		}
@@ -509,7 +504,7 @@ static int set_fatent_value(fsdata *mydata,
 __u32 entry,
 __u32 entry_value) return -1;
 		}
 
-		if (disk_read(startblock, getsize, bufptr) < 0) {
+		if (fat_disk_read(startblock, getsize, bufptr) < 0) {
 			debug("Error reading FAT blocks\n");
 			return -1;
fs/fat/fat_write.c +0 -0
 		}
@@ -793,7 +788,7 @@ static int check_overflow(fsdata *mydata, __u32
 clustnum, unsigned long size) if (size % mydata->sect_size)
 		sect_num++;
 
Quoted 2 lines

fs/fat/fat_write.c static int check_overflow(fsdata *mydata, __u32 clustnum, unsigned long size) · line 791

Quoted 2 lines
fs/fat/fat_write.c +1 -1
-	if (startsect + sect_num > cur_part_info.start + total_sector)
+	if (startsect + sect_num > total_sector)

Are you certain? It depends on if mydata->data_begin and mydata->rootdir_sect are disk- or partition-relative (I did not check).

Anyway, this is a change of behavior, so that should go into a separate patch.

Quoted 15 lines
Quoted 15 lines
fs/fat/fat_write.c +0 -0
 		return -1;
 
 	return 0;
@@ -932,14 +927,14 @@ static int do_fat_write(const char *filename,

void
*buffer,

fs/fat/fat_write.c +1 -1
 	dir_curclust = 0;
 
-	if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
+	if (fat_read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) {
 		debug("error: reading boot sector\n");
 		return -1;
 	}
Quoted 4 lines

fs/fat/fat_write.c static int do_fat_write(const char *filename, void *buffer, · line 937

Quoted 4 lines
fs/fat/fat_write.c +1 -1
 	total_sector = bs.total_sect;
 	if (total_sector == 0)
-		total_sector = cur_part_info.size;
+		total_sector = fat_get_partsize();

cur_dev was not tested here in the original code.

Quoted 20 lines
Quoted 20 lines
fs/fat/fat_write.c +0 -0
 	if (mydata->fatsize == 32)
 		mydata->fatlength = bs.fat32_length;
@@ -977,7 +972,7 @@ static int do_fat_write(const char *filename,

void
*buffer, return -1;

fs/fat/fat_write.c +1 -1
 	}
 
-	if (disk_read(cursect,
+	if (fat_disk_read(cursect,
 		(mydata->fatsize == 32) ?
 		(mydata->clust_size) :
 		PREFETCH_BLOCKS, do_fat_read_block) < 0) {
include/fat.h +0 -0
index f1b4a0d..98f77a5 100644
@@ -213,4 +213,11 @@ const char *file_getfsname(int idx);
 int fat_register_device(block_dev_desc_t *dev_desc, int part_no);
 
Quoted 11 lines

include/fat.h const char *file_getfsname(int idx); · line 223

Quoted 11 lines
include/fat.h +0 -0
 int file_fat_write(const char *filename, void *buffer, unsigned
 long

maxsize); +

include/fat.h +4 -0
+/*internal functions for fs/blockdev API abstraction*/
+int fat_disk_read(__u32 block, __u32 nr_blocks, void *buf);
+int fat_disk_write(__u32 block, __u32 nr_blocks, void *buf);
+int fat_read_bootsectandvi(boot_sector *bs, volume_info *vi, int

*fatsize);

include/fat.h +2 -0
+int fat_get_blksz(void);
+int fat_get_partsize(void);
 #endif /* _FAT_H_ */

Since these are FAT-private functions, shouldn't their declarations go to some private header file in the fs/fat/ folder?

Best regards,
Beno?t