[PATCH 2/6] fat: Separate fat.c from fat_write.c
Maarten Brock
Maarten.Brock at sttls.nl
Fri Nov 14 11:55:17 CET 2025
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 <u-boot-bounces at lists.denx.de> On Behalf Of Heinrich Schuchardt
> Sent: Thursday, 13 November 2025 07:47
> To: Tom Rini <trini at konsulko.com>
> Cc: Simon Glass <simon.glass at canonical.com>; Gabriel Dalimonte
> <gabriel.dalimonte at gmail.com>; AKASHI Takahiro <akashi.tkhro at gmail.com>; u-
> boot at lists.denx.de; Claude <noreply at anthropic.com>; Heinrich Schuchardt
> <heinrich.schuchardt at canonical.com>
> Subject: [PATCH 2/6] fat: Separate fat.c from fat_write.c
>
> From: Simon Glass <simon.glass at canonical.com>
>
> 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 <noreply at anthropic.com>
> Signed-off-by: Simon Glass <simon.glass at canonical.com>
> Tested-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> ---
> 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
More information about the U-Boot
mailing list