Hello Heinrich, Don't you run the risk of getting into namespace problems when you make these static functions and global variables non-static? Esp. the ones without the word 'fat' in them seem to be in danger here.
E.g. downcase(), cur_dev, disk_read() Kind Regards, Maarten > -----Original Message----- > From: U-Boot <[email protected]> On Behalf Of Heinrich Schuchardt > Sent: Thursday, 13 November 2025 07:47 > To: Tom Rini <[email protected]> > Cc: Simon Glass <[email protected]>; Gabriel Dalimonte > <[email protected]>; AKASHI Takahiro <[email protected]>; u- > [email protected]; Claude <[email protected]>; Heinrich Schuchardt > <[email protected]> > Subject: [PATCH 2/6] fat: Separate fat.c from fat_write.c > > From: Simon Glass <[email protected]> > > Currently fat_write.c includes fat.c directly, which is unusual and > makes the code harder to maintain. Use the internal header file to hold > shared functions, to avoid this. > > Co-developed-by: Claude <[email protected]> > Signed-off-by: Simon Glass <[email protected]> > Tested-by: Heinrich Schuchardt <[email protected]> > --- > fs/fat/Makefile | 2 +- > fs/fat/fat.c | 91 ++++++----------------------------------- > fs/fat/fat_internal.h | 95 +++++++++++++++++++++++++++++++++++++++++-- > fs/fat/fat_write.c | 5 ++- > 4 files changed, 108 insertions(+), 85 deletions(-) > > diff --git a/fs/fat/Makefile b/fs/fat/Makefile > index 3e739044537..f3982fca4c6 100644 > --- a/fs/fat/Makefile > +++ b/fs/fat/Makefile > @@ -2,4 +2,4 @@ > # > > obj-$(CONFIG_$(PHASE_)FS_FAT) = fat.o > -obj-$(CONFIG_$(PHASE_)FAT_WRITE) = fat_write.o > +obj-$(CONFIG_$(PHASE_)FAT_WRITE) += fat_write.o > diff --git a/fs/fat/fat.c b/fs/fat/fat.c > index 5114e97e924..0fdbd411901 100644 > --- a/fs/fat/fat.c > +++ b/fs/fat/fat.c > @@ -33,7 +33,7 @@ > * 'len' may be larger than the length of 'str' if 'str' is NULL > * terminated. > */ > -static void downcase(char *str, size_t len) > +void downcase(char *str, size_t len) > { > while (*str != '\0' && len--) { > *str = tolower(*str); > @@ -41,10 +41,10 @@ static void downcase(char *str, size_t len) > } > } > > -static struct blk_desc *cur_dev; > -static struct disk_partition cur_part_info; > +struct blk_desc *cur_dev; > +struct disk_partition cur_part_info; > > -static int disk_read(__u32 block, __u32 nr_blocks, void *buf) > +int disk_read(__u32 block, __u32 nr_blocks, void *buf) > { > ulong ret; > > @@ -145,8 +145,6 @@ static void get_name(dir_entry *dirent, char *s_name) > *s_name = DELETED_FLAG; > } > > -static int flush_dirty_fat_buffer(fsdata *mydata); > - > #if !CONFIG_IS_ENABLED(FAT_WRITE) > /* Stub for read only operation */ > int flush_dirty_fat_buffer(fsdata *mydata) > @@ -160,7 +158,7 @@ int flush_dirty_fat_buffer(fsdata *mydata) > * Get the entry at index 'entry' in a FAT (12/16/32) table. > * On failure 0x00 is returned. > */ > -static __u32 get_fatent(fsdata *mydata, __u32 entry) > +__u32 get_fatent(fsdata *mydata, __u32 entry) > { > __u32 bufnum; > __u32 offset, off8; > @@ -469,7 +467,7 @@ static int slot2str(dir_slot *slotptr, char *l_name, int > *idx) > } > > /* Calculate short name checksum */ > -static __u8 mkcksum(struct nameext *nameext) > +__u8 mkcksum(struct nameext *nameext) > { > int i; > u8 *pos = (void *)nameext; > @@ -699,17 +697,7 @@ static int get_fs_info(fsdata *mydata) > return 0; > } > > -static int fat_itr_isdir(fat_itr *itr); > - > -/** > - * fat_itr_root() - initialize an iterator to start at the root > - * directory > - * > - * @itr: iterator to initialize > - * @fsdata: filesystem data for the partition > - * Return: 0 on success, else -errno > - */ > -static int fat_itr_root(fat_itr *itr, fsdata *fsdata) > +int fat_itr_root(fat_itr *itr, fsdata *fsdata) > { > if (get_fs_info(fsdata)) > return -ENXIO; > @@ -726,24 +714,7 @@ static int fat_itr_root(fat_itr *itr, fsdata *fsdata) > return 0; > } > > -/** > - * fat_itr_child() - initialize an iterator to descend into a sub- > - * directory > - * > - * Initializes 'itr' to iterate the contents of the directory at > - * the current cursor position of 'parent'. It is an error to > - * call this if the current cursor of 'parent' is pointing at a > - * regular file. > - * > - * Note that 'itr' and 'parent' can be the same pointer if you do > - * not need to preserve 'parent' after this call, which is useful > - * for traversing directory structure to resolve a file/directory. > - * > - * @itr: iterator to initialize > - * @parent: the iterator pointing at a directory entry in the > - * parent directory of the directory to iterate > - */ > -static void fat_itr_child(fat_itr *itr, fat_itr *parent) > +void fat_itr_child(fat_itr *itr, fat_itr *parent) > { > fsdata *mydata = parent->fsdata; /* for silly macros */ > unsigned clustnum = START(parent->dent); > @@ -844,7 +815,7 @@ void *fat_next_cluster(fat_itr *itr, unsigned int *nbytes) > return itr->block; > } > > -static dir_entry *next_dent(fat_itr *itr) > +dir_entry *next_dent(fat_itr *itr) > { > if (itr->remaining == 0) { > unsigned nbytes; > @@ -920,16 +891,7 @@ static dir_entry *extract_vfat_name(fat_itr *itr) > return dent; > } > > -/** > - * fat_itr_next() - step to the next entry in a directory > - * > - * Must be called once on a new iterator before the cursor is valid. > - * > - * @itr: the iterator to iterate > - * Return: boolean, 1 if success or 0 if no more entries in the > - * current directory > - */ > -static int fat_itr_next(fat_itr *itr) > +int fat_itr_next(fat_itr *itr) > { > dir_entry *dent; > > @@ -992,41 +954,12 @@ static int fat_itr_next(fat_itr *itr) > return 1; > } > > -/** > - * fat_itr_isdir() - is current cursor position pointing to a directory > - * > - * @itr: the iterator > - * Return: true if cursor is at a directory > - */ > -static int fat_itr_isdir(fat_itr *itr) > +int fat_itr_isdir(fat_itr *itr) > { > return !!(itr->dent->attr & ATTR_DIR); > } > > -/* > - * Helpers: > - */ > - > -#define TYPE_FILE 0x1 > -#define TYPE_DIR 0x2 > -#define TYPE_ANY (TYPE_FILE | TYPE_DIR) > - > -/** > - * fat_itr_resolve() - traverse directory structure to resolve the > - * requested path. > - * > - * Traverse directory structure to the requested path. If the specified > - * path is to a directory, this will descend into the directory and > - * leave it iterator at the start of the directory. If the path is to a > - * file, it will leave the iterator in the parent directory with current > - * cursor at file's entry in the directory. > - * > - * @itr: iterator initialized to root > - * @path: the requested path > - * @type: bitmask of allowable file types > - * Return: 0 on success or -errno > - */ > -static int fat_itr_resolve(fat_itr *itr, const char *path, unsigned type) > +int fat_itr_resolve(fat_itr *itr, const char *path, unsigned int type) > { > const char *next; > > diff --git a/fs/fat/fat_internal.h b/fs/fat/fat_internal.h > index 0174cd611e7..10881a15569 100644 > --- a/fs/fat/fat_internal.h > +++ b/fs/fat/fat_internal.h > @@ -22,6 +22,14 @@ struct disk_partition; > #define DOS_FS_TYPE_OFFSET 0x36 > #define DOS_FS32_TYPE_OFFSET 0x52 > > +#define TYPE_FILE 0x1 > +#define TYPE_DIR 0x2 > +#define TYPE_ANY (TYPE_FILE | TYPE_DIR) > + > +/* Global variables shared between fat.c and fat_write.c */ > +extern struct blk_desc *cur_dev; > +extern struct disk_partition cur_part_info; > + > /** > * struct fat_itr - directory iterator, to simplify filesystem traversal > * > @@ -105,8 +113,89 @@ struct fat_itr { > u8 block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN); > }; > > -#define TYPE_FILE 0x1 > -#define TYPE_DIR 0x2 > -#define TYPE_ANY (TYPE_FILE | TYPE_DIR) > +/** > + * downcase() - convert a string to lowercase > + * @str: string to convert > + * @len: maximum number of characters to convert > + */ > +void downcase(char *str, size_t len); > + > +/** > + * next_dent() - get next directory entry > + * @itr: directory iterator > + * Return: pointer to next directory entry, or NULL if at end > + */ > +dir_entry *next_dent(fat_itr *itr); > + > +/** > + * disk_read() - read sectors from the current FAT device > + * @block: logical block number > + * @nr_blocks: number of blocks to read > + * @buf: buffer to read data into > + * Return: number of blocks read, -1 on error > + */ > +int disk_read(__u32 block, __u32 nr_blocks, void *buf); > + > +/** > + * flush_dirty_fat_buffer() - write fat buffer to disk if dirty > + * @mydata: filesystem data > + * Return: 0 on success, -1 on error > + */ > +int flush_dirty_fat_buffer(fsdata *mydata); > + > +/* Internal function declarations */ > + > +/** > + * get_fatent() - get the entry at index 'entry' in a FAT (12/16/32) table > + * @mydata: filesystem data > + * @entry: FAT entry index > + * Return: FAT entry value, 0x00 on failure > + */ > +__u32 get_fatent(fsdata *mydata, __u32 entry); > + > +/** > + * mkcksum() - calculate short name checksum > + * @nameext: name and extension structure > + * Return: checksum value > + */ > +__u8 mkcksum(struct nameext *nameext); > + > +/** > + * fat_itr_root() - initialize an iterator to start at the root directory > + * @itr: iterator to initialize > + * @fsdata: filesystem data for the partition > + * Return: 0 on success, else -errno > + */ > +int fat_itr_root(fat_itr *itr, fsdata *fsdata); > + > +/** > + * fat_itr_child() - initialize an iterator to descend into a sub-directory > + * @itr: iterator to initialize > + * @parent: the iterator pointing at a directory entry in the parent > directory > + */ > +void fat_itr_child(fat_itr *itr, fat_itr *parent); > + > +/** > + * fat_itr_next() - step to the next entry in a directory > + * @itr: the iterator to iterate > + * Return: 1 if success or 0 if no more entries in the current directory > + */ > +int fat_itr_next(fat_itr *itr); > + > +/** > + * fat_itr_isdir() - is current cursor position pointing to a directory > + * @itr: the iterator > + * Return: true if cursor is at a directory > + */ > +int fat_itr_isdir(fat_itr *itr); > + > +/** > + * fat_itr_resolve() - traverse directory structure to resolve the requested > path > + * @itr: iterator initialized to root > + * @path: the requested path > + * @type: bitmask of allowable file types > + * Return: 0 on success or -errno > + */ > +int fat_itr_resolve(fat_itr *itr, const char *path, uint type); > > #endif /* _FAT_INTERNAL_H_ */ > diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c > index 45a2eef712b..638a6223700 100644 > --- a/fs/fat/fat_write.c > +++ b/fs/fat/fat_write.c > @@ -13,15 +13,16 @@ > #include <fat.h> > #include <log.h> > #include <malloc.h> > +#include <memalign.h> > #include <part.h> > #include <rand.h> > +#include <rtc.h> > #include <asm/byteorder.h> > #include <asm/cache.h> > #include <dm/uclass.h> > #include <linux/ctype.h> > #include <linux/math64.h> > #include "fat_internal.h" > -#include "fat.c" > > static dir_entry *find_directory_entry(fat_itr *itr, char *filename); > static int new_dir_table(fat_itr *itr); > @@ -216,7 +217,7 @@ static int disk_write(__u32 block, __u32 nr_blocks, void > *buf) > /* > * Write fat buffer into block device > */ > -static int flush_dirty_fat_buffer(fsdata *mydata) > +int flush_dirty_fat_buffer(fsdata *mydata) > { > int getsize = FATBUFBLOCKS; > __u32 fatlength = mydata->fatlength; > -- > 2.51.0
