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

[Michael Nazzareno Trimarchi]

Hi Tom

On Sun, Nov 3, 2024 at 9:21 AM Michael Nazzareno Trimarchi
<michael at amarulasolutions.com> wrote:
>
> Hi
>
> Il dom 3 nov 2024, 00:53 Tom Rini <trini at konsulko.com> ha scritto:
>>
>> On Sun, Nov 03, 2024 at 12:36:38AM +0100, 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?
>>
>> ... I always forget b4 compares with checked out branch, that should
>> have been master.
>>
>> > int ext4fs_opendir(const char *dirname, struct fs_dir_stream **dirsp) {
>> >     struct ext4_dir_stream *dirs = NULL;
>> >     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) {
>> >         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);
>
>
> I have add in this path the free of dirs, because according to the code I can see from this email is not reference anymore and anyway according to what I have commented already moving allocation of dirs later make the code a bit simpler.
>

There are few leaks in this function. I have fixed, can you take a
look again on what you merge?

Michael

> Michael
>
>
>> >     }
>> >
>> >     if (dir)
>> >         ext4fs_free_node(dir, &ext4fs_root->diropen);
>> >
>> >     return ret;
>> > }
>> >
>> > Should not be like this?
>>
>> Please elaborate?
>>
>> --
>> 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