Re: [LTP] [PATCH v4] Add stat04 test

[Petr Vorel <pvorel@suse.cz>]

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


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp