Re: [PATCH 3/9] fs: ext4: print change date in directory listing

Hi Heinrich,

On Wed, 20 May 2026, 19:46 Heinrich Schuchardt,
<[email protected]> wrote:
>
> On 5/20/26 22:42, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Mon, 18 May 2026 at 00:57, Heinrich Schuchardt
> > <[email protected]> wrote:
> >>
> >> Declare FS_CAP_DATE in the ext4 fstype_info entry so that fs_ls_generic()
> >> displays the modification date alongside the file size:
> >>
> >>   4096 2024-03-15 09:30 filename.txt
> >>
> >> Signed-off-by: Heinrich Schuchardt <[email protected]>
> >> ---
> >>   fs/fs.c | 3 +++
> >>   1 file changed, 3 insertions(+)
> >>
> >> diff --git a/fs/fs.c b/fs/fs.c
> >> index f8e4794c10e..482a5523712 100644
> >> --- a/fs/fs.c
> >> +++ b/fs/fs.c
> >> @@ -261,6 +261,9 @@ static struct fstype_info fstypes[] = {
> >>                  .fstype = FS_TYPE_EXT,
> >>                  .name = "ext4",
> >>                  .null_dev_desc_ok = false,
> >> +#if !IS_ENABLED(CONFIG_XPL_BUILD)
> >> +               .caps = FS_CAP_DATE,
> >> +#endif
> >>                  .probe = ext4fs_probe,
> >>                  .close = ext4fs_close,
> >>                  .ls = fs_ls_generic,
> >> --
> >> 2.53.0
> >>
> >
> > I would prefer having a head-file macro which expands to nothing for
> > xPL builds, rather than adding preprocessor macros.
> >
> > Regards,
> > Simon
>
> Hello Simon,
>
> In the internet I could not find what a "head-file macro" might be.

Sorry I meant 'header-file macro'

>
> As struct fstype_info is not defined in a header file, a preprocessor
> macro defined in a header file would not make sense here.
>
> Do you mean something like:
>
> #if IS_ENABLED(CONFIG_XPL_BUILD)
> #define FS_CAPS(flags)  /* empty */
> #else
> #define FS_CAPS(flags)  .caps = (flags),
> #endif
>
> static struct fstype_info fstypes[] = {
> #if CONFIG_IS_ENABLED(FS_FAT)
>          {
>                  .fstype = FS_TYPE_FAT,
>                  .name = "fat",
>                  .null_dev_desc_ok = false,
>                  FS_CAPS(FS_CAP_DATE)
>                  .probe = fat_set_blk_dev,
> ...
>
> A line without a comma in the initializer is easily mistaken as
> incorrect. I am not sure that a code reviewers life is made easier with
> defining a new preprocessor macro.

Firstly I wonder if it is actually worth making the field conditional?
We have a null_dev_desc_ok bool which could be turned into flags, if
you are trying to save space.

Ideally we would have a new Kconfig for this, so it is possible to
enable the feature in particular xPL builds.

If you don't like the example above, another option is:

CONFIG_IS_ENABLED(YOUR_OPTION, (FS_CAP_DATE,))

which we use in quite a few places now.

Regards,
Simon