[LTP] [PATCH v4] Add stat04 test
Petr Vorel
pvorel@suse.cz
Fri Feb 23 16:47:32 CET 2024
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?
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
More information about the ltp
mailing list