Re: [LTP] [PATCH 1/5] hugemmap: Move MNTPOINT definition to header

[Petr Vorel <pvorel@suse.cz>]

Hi Cyril,

first, thanks a lot for looking into this.

> Hi!
> > Also, change it from "hugetlbfs/" to the usual "mntpoint".

> Isn't that actually a regression? The name was more descriptive when it
> was hugetlbfs/...

I agree with a readability point.

From following library code in lib/tst_test.c I expect we mount only once:

	if (!!tst_test->needs_rofs + !!tst_test->needs_devfs +
	    !!tst_test->needs_device + !!tst_test->needs_hugetlbfs > 1) {
		tst_brk(TBROK,
			"Two or more of needs_{rofs, devfs, device, hugetlbfs} are set");
	}

Therefore it's really only readability point due .needs_hugetlbfs (but again,
I agree with it, I guess you want to see the path in the test output and dmesg
logs or when directory is not umounted due bug, right?)

Is there any other .mntpoint value which should be preserved as a different
(I suppose no, but maybe I overlooked)?

BTW without this specific hugetlbfs case I would even suggest to use always
MNTPOINT in whole source tree and remove tst_test->mntpoint (but that might be
also step back - readability might suffer).

> I guess that it makes sense to move the MNTPOINT to the hugetlb.h but I
> would keep the actual directory name the same...

E.g. in hugetlb.h:

#define MNTPOINT "hugetlbfs/"

nit: IMHO it can be just "hugetlbfs"? tst_creat_unlinked calls:

	snprintf(template, PATH_MAX, "%s/ltp_%.3sXXXXXX",
			path, tid);

=> there is / after %s which is path parameter.

... and in tst_test.h:

#ifndef MNTPOINT
# define MNTPOINT "mntpoint"
#endif

This should be safe, because hugetlb.h includes tst_test.h.

> I would have defined MNTPOINT in the tst_test.h rather than in tst_fs.h.
(from your other email)

But what about OVL_BASE_MNTPOINT in tst_fs.h? I guess all of these should be on
the same place, right? (in 1st commit)

Kind regards,
Petr

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