Re: [LTP] [PATCH v2 1/5] Add stat04 test

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> PATH_MAX is 4096, right? Is it really needed to test the length?

Contrary to the popular belief PATH_MAX is a limit for lenght of a
single filesytem path component, i.e. directory or a file name. And yes
I've seen bugreports for LTP where people pointed TMPDIR to a deep
enough filesystem path for this to matter.

Anyways I've proposed to allocate the paths instead with asprintf().

> > +
> > +	/* change st_blksize / st_dev */
> > +	SAFE_STAT(".", &sb);
> > +	pagesize = sb.st_blksize == 4096 ? 1024 : 4096;
> > +
> > +	snprintf(opt_bsize, sizeof(opt_bsize), "-b %i", pagesize);
> > +	SAFE_MKFS(tst_device->dev, tst_device->fs_type, fs_opts, NULL);
> > +	SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, 0);
> 
> Isn't symlink filesystem related? Shouldn't this be run on all_filesystems?

Hmm, I was assuming that symlink was a generic funcionality, but
thinking about it the symlink data has to be stored in the filesystem so
I suppose that there is some filesystem specific code to be tested after
all.

I guess that this could stil be implemented but we would have to add
support for multiple devices. I can have a look later on, but I wouldn't
block this patch because of that.

> > +
> > +	SAFE_TOUCH(FILENAME, 0777, NULL);
> > +
> > +	/* change st_nlink */
> > +	SAFE_LINK(FILENAME, "linked_file");
> > +
> > +	/* change st_uid and st_gid */
> > +	SAFE_CHOWN(FILENAME, 1000, 1000);
> > +
> > +	/* change st_size */
> > +	fd = SAFE_OPEN(FILENAME, O_WRONLY, 0777);
> > +	tst_fill_fd(fd, 'a', TST_KB, 500);
> > +	SAFE_CLOSE(fd);
> > +
> > +	/* change st_atime / st_mtime / st_ctime */
> > +	usleep(1001000);
> > +
> > +	memset(file_path, 0, PATH_MAX);
> > +	snprintf(file_path, PATH_MAX, "%s/%s", tmpdir, FILENAME);
> > +
> > +	memset(symb_path, 0, PATH_MAX);
> > +	snprintf(symb_path, PATH_MAX, "%s/%s", tmpdir, SYMBNAME);
> > +
> > +	SAFE_SYMLINK(file_path, symb_path);
> > +}
> > +
> > +static void cleanup(void)
> > +{
> > +	free(tmpdir);
> nit: I know that tst_get_tmpdir() is first thing in setup(), but I would still
> guard it with if (tmpdir) (code may change later).
> 
> > +
> > +	SAFE_UNLINK(SYMBNAME);
> nit: Ideally this would be guarded by flag that SAFE_SYMLINK(file_path,
> symb_path) got executed.

I do not think that we need to unlink the symlink, it should be removed
by the test library when the temorary directory is removed. Or is there
a problem with that?

-- 
Cyril Hrubis
chrubis@suse.cz

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