[LTP] [PATCH v4] Add stat04 test

[Andrea Cervesato]

Hi,

> On 23 Feb 2024, at 16:47, Petr Vorel <pvorel@suse.cz> wrote:
> 
> Hi Andrea,
> 
>>>> +    char *symname = "my_symlink0";
>>>> +    char *tmpdir = tst_get_tmpdir();
>>>> +
>>>> +    SAFE_SYMLINK(tmpdir, symname);
>>>> +
>>>> +    struct stat path;
>>>> +    struct stat link;
>>> nit: maybe define struct at the top of the function?
>> This is common in the first versions of C, but a good practice is to define
>> variables as close as possible where they are used in order to improve
>> readability.
> 
> Fair enough.
> 
>>>> +
>>>> +    TST_EXP_PASS(stat(tmpdir, &path));
>>>> +    free(tmpdir);
>>> If SAFE_SYMLINK() fails, free() will not happen. I wonder if we should bother
>>> (we'd have to set it NULL and add a cleanup function).
> 
>>> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> 
>>> Kind regards,
>>> Petr
> 
> ...
> 
>> According to Cyril suggestions we are probably done with this patch so it
>> can be merged. Isn't it?
> 
> Well you free() later in the function. But Cyril's  note sounds reasonable for
> me:
>    "call it once in the test setup() or use "." instead."
> 
> Could you please do it?
> 
Makes sense. I think we should adapt all other tests as well because it’s full of memory leakage when it comes to get temporary directory.

> Also most of the tests really run tst_get_tmpdir() in the setup() (or cleanup(),
> there are exceptions (new API: testcases/kernel/syscalls/pathconf/pathconf01.c,
> old API: testcases/kernel/syscalls/symlink/symlink01.c,
> testcases/kernel/syscalls/clone/clone02.c), which should be fixed.
> IMHO tests which call tst_get_tmpdir() in the run()
> 
> Kind regards,
> Petr
> 
>> Thanks,
>> Andrea
> 

Regards,
Andrea