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

[Heinrich Schuchardt]

On 11/7/24 10:31, Michael Nazzareno Trimarchi wrote:
> 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

Coverity probably would have reported this. But I think we only run it
on tags.

This article describes how Valgrind could be used:

https://developers.redhat.com/articles/2022/03/23/use-valgrind-memcheck-custom-memory-manager

Both malloc and LMB would be candidates.

Best regards

Heinrich

>
> 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
>>>>
>>>>
>>>
>>>
>>
>
>