[PATCH 0/5] fs: ext4: implement opendir, readdir, closedir

[Michael Nazzareno Trimarchi]

Hi

On Thu, Nov 7, 2024 at 10:27 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 11/3/24 00:36, Michael Nazzareno Trimarchi wrote:
> > Hi Tom
> >
> > On Sun, Nov 3, 2024 at 12:26 AM Tom Rini <trini at konsulko.com> wrote:
> >>
> >> On Sat, 26 Oct 2024 08:40:43 +0200, Heinrich Schuchardt wrote:
> >>
> >>> With this series opendir, readdir, closedir are implemented for ext4.
> >>> These functions are needed for the UEFI sub-system to interact with
> >>> the ext4 file system.
> >>>
> >>> To reduce code growth the functions are reused to implement the ls
> >>> command for ext4.
> >>>
> >>> [...]
> >>
> >> Applied to local tree/v2-tidy-test-dir, thanks!
> >>
> > Am I sleeping?
> >
> > int ext4fs_opendir(const char *dirname, struct fs_dir_stream **dirsp) {
> >      struct ext4_dir_stream *dirs = NULL;
>
> Thank you for your review.
>
> We should not set a value that we do not use.
>
> >      struct ext2fs_node *dir = NULL;
> >      int ret;
> >
> >      *dirsp = NULL;
> >
> >      dirs = calloc(1, sizeof(struct ext4_dir_stream));
> >      if (!dirs)
> >          return -ENOMEM;
> >
> >      dirs->dirname = strdup(dirname);
> >      if (!dirs->dirname) {
>
> Yes this line in my code is incorrect.
>

We need to use valgrind in unit test or we need some code analys to
automate this process.
We are always looking for memory leaks in uboot in exit path or we
need to have alloction funciton that
automatic free the memory when we go in error path

Michael

> >          free(dirs);
> >          return -ENOMEM;
> >      }
> >
> >      ret = ext4fs_find_file(dirname, &ext4fs_root->diropen, &dir,
> > FILETYPE_DIRECTORY);
> >
> >      if (ret == 1) {
> >          ret = 0;
> >          *dirsp = (struct fs_dir_stream *)dirs;
> >      } else {
> >          ret = -ENOENT;
> >          free(dirs->dirname);
> >          free(dirs);
>
> Yes, we should avoid a memory leak here.
>
> I will send a patch.
>
> Best regards
>
> Heinrich
>
> >      }
> >
> >      if (dir)
> >          ext4fs_free_node(dir, &ext4fs_root->diropen);
> >
> >      return ret;
> > }
> >
> > Should not be like this?
> >
> >> --
> >> Tom
> >>
> >>
> >
> >
>


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael at amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info at amarulasolutions.com
www.amarulasolutions.com