Hi Heinrich,
On 2026-10-12T14:00:39, Heinrich Schuchardt
<[email protected]> wrote:
> fs: print change date in directory listing for FAT
>
> fs_ls_generic() displays file sizes but no timestamps. The FAT
> filesystem stores a change date in every directory entry and already
> populates fs_dirent::change_time in fat_readdir(). Print the date
> alongside the file size for filesystems that provide it.
>
> Add a u32 capability bitmap (caps) to struct fstype_info. Each bit
> documents a property that the filesystem's readdir() implementation
> guarantees:
>
> FS_CAP_DATE BIT(0) change_time in fs_dirent is valid
>
> fs_ls_generic() tests FS_CAP_DATE once before the loop to select a
> consistent output format for the entire listing:
>
> 12345678 2024-03-15 09:30 filename.txt (FAT)
> 12345678 filename.txt (ext4, squashfs, ...)
>
> Set FS_CAP_DATE for FAT. fat2rtc() loses the __maybe_unused annotation
> [...]
>
> fs/fat/fat.c | 2 +-
> fs/fs.c | 38 +++++++++++++++++++++++++++++++++++---
> 2 files changed, 36 insertions(+), 4 deletions(-)
> Set FS_CAP_DATE for FAT. fat2rtc() loses the __maybe_unused annotation
> since it is now called unconditionally outside XPL builds.
This patch doesn't remove the annotation. fat2rtc() is still 'static
void __maybe_unused' in fat.c. Please can you either remove the
annotation or drop this sentence?
> diff --git a/fs/fs.c b/fs/fs.c
> @@ -38,6 +38,11 @@ static int fs_dev_part;
> +/*/
> + * define FS_CAP_DATE - readdir() populates fs_dirent::change_time
> + */
The opener '/*/' looks like a typo for '/**'
> diff --git a/fs/fs.c b/fs/fs.c
> @@ -50,6 +55,10 @@ struct fstype_info {
> +#if !IS_ENABLED(CONFIG_XPL_BUILD)
> + /* Capability flags (FS_CAP_*) */
> + u32 caps;
> +#endif
This #if leads to three #if blocks in this patch, plus one more for
each filesystem that gets the cap (patch 3 adds another). All of that
saves only 4 bytes per entry. I suggest always including caps and
doing this in fs_ls_generic():
bool has_date = !IS_ENABLED(CONFIG_XPL_BUILD) &&
(fs_get_info(fs_type)->caps & FS_CAP_DATE);
The compiler still drops the date code in XPL builds, and fstypes[]
needs no #ifdefs. What do you think?
Even better IMO you could create an accessor in the header file as we
have done in global_data.h
> diff --git a/fs/fs.c b/fs/fs.c
> @@ -112,15 +130,26 @@ static int fs_ls_generic(const char *dirname)
> - printf(" %s/\n", dent->name);
> + printf(" ");
> ndirs++;
> } else if (dent->type == FS_DT_LNK) {
> - printf(" <SYM> %s\n", dent->name);
> + printf(" <SYM> ");
> nfiles++;
> } else {
> - printf(" %8lld %s\n", dent->size, dent->name);
> + printf(" %8lld ", dent->size);
This also changes the output for every filesystem without FS_CAP_DATE:
the name column moves from 12 to 10 characters. That is why patch 9
has to change the erofs test, and user scripts that parse ls output
could break too. Please can you keep the existing three-space gap when
has_date is false, e.g. by printing the separator after the optional
date? Then only FAT and ext4 output changes.
Regards,
Simon
