* [LTP] [PATCH] tmpdir: rename tst_tmpdir_mkpath to tst_tmpdir_genpath
@ 2024-09-10 7:10 Li Wang
2024-09-10 8:52 ` Cyril Hrubis
0 siblings, 1 reply; 6+ messages in thread
From: Li Wang @ 2024-09-10 7:10 UTC (permalink / raw)
To: ltp
When I reviewed the rename15 patch I felt confused a while on
function like tst_tmpdir_mkpath (reminds me of: mkdir).
Because the name could be misleading since it suggests that a
file or directory is being created, when in fact it is simply
constructing a path inside a temporary directory without
actually creating any files.
To make the function's purpose clearer, the name should reflect
that it is only constructing or generating a path, not creating
any files or directories.
So I think either 'tst_tmpdir_genpath' or 'tst_tmpdir_buildpath'
would be concise and clear options.
Signed-off-by: Li Wang <liwang@redhat.com>
---
include/tst_tmpdir.h | 4 ++--
lib/newlib_tests/tst_device.c | 2 +-
lib/tst_tmpdir.c | 2 +-
testcases/kernel/syscalls/faccessat/faccessat01.c | 2 +-
testcases/kernel/syscalls/faccessat2/faccessat201.c | 2 +-
testcases/kernel/syscalls/fchmodat/fchmodat01.c | 2 +-
testcases/kernel/syscalls/ioctl/ioctl_loop01.c | 2 +-
testcases/kernel/syscalls/ioctl/ioctl_loop02.c | 4 ++--
testcases/kernel/syscalls/ioctl/ioctl_loop05.c | 2 +-
testcases/kernel/syscalls/mount/mount06.c | 8 ++++----
testcases/kernel/syscalls/mount/mount07.c | 6 +++---
testcases/kernel/syscalls/pathconf/pathconf02.c | 2 +-
testcases/kernel/syscalls/readlinkat/readlinkat01.c | 2 +-
testcases/kernel/syscalls/stat/stat04.c | 4 ++--
14 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/include/tst_tmpdir.h b/include/tst_tmpdir.h
index d25eeba02..85d708233 100644
--- a/include/tst_tmpdir.h
+++ b/include/tst_tmpdir.h
@@ -29,7 +29,7 @@ void tst_purge_dir(const char *path);
char *tst_tmpdir_path(void);
/**
- * tst_tmpdir_mkpath - Construct an absolute path pointing to a file inside tmpdir.
+ * tst_tmpdir_genpath - Construct an absolute path pointing to a file inside tmpdir.
*
* Constructs a path inside tmpdir i.e. adds a prefix pointing to the current
* test tmpdir to the string build by the printf-like format.
@@ -41,7 +41,7 @@ char *tst_tmpdir_path(void);
* of the test. If allocation fails the function calls tst_brk() and exits the
* test.
*/
-char *tst_tmpdir_mkpath(const char *fmt, ...)
+char *tst_tmpdir_genpath(const char *fmt, ...)
__attribute__((format(printf, 1, 2)));
/*
diff --git a/lib/newlib_tests/tst_device.c b/lib/newlib_tests/tst_device.c
index ef69728f4..a450b284d 100644
--- a/lib/newlib_tests/tst_device.c
+++ b/lib/newlib_tests/tst_device.c
@@ -29,7 +29,7 @@ static void setup(void)
{
int fd;
- mntpoint = tst_tmpdir_mkpath("mnt");
+ mntpoint = tst_tmpdir_genpath("mnt");
fd = SAFE_OPEN(tst_device->dev, O_RDONLY);
diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
index 5c63f7b3c..6ed2367b9 100644
--- a/lib/tst_tmpdir.c
+++ b/lib/tst_tmpdir.c
@@ -372,7 +372,7 @@ char *tst_tmpdir_path(void)
return tmpdir;
}
-char *tst_tmpdir_mkpath(const char *fmt, ...)
+char *tst_tmpdir_genpath(const char *fmt, ...)
{
size_t testdir_len, path_len;
va_list va, vac;
diff --git a/testcases/kernel/syscalls/faccessat/faccessat01.c b/testcases/kernel/syscalls/faccessat/faccessat01.c
index d429cdd14..6be2b4bb7 100644
--- a/testcases/kernel/syscalls/faccessat/faccessat01.c
+++ b/testcases/kernel/syscalls/faccessat/faccessat01.c
@@ -55,7 +55,7 @@ static void verify_faccessat(unsigned int i)
static void setup(void)
{
- abs_path = tst_tmpdir_mkpath(FILEPATH);
+ abs_path = tst_tmpdir_genpath(FILEPATH);
SAFE_MKDIR(TESTDIR, 0700);
dir_fd = SAFE_OPEN(TESTDIR, O_DIRECTORY);
diff --git a/testcases/kernel/syscalls/faccessat2/faccessat201.c b/testcases/kernel/syscalls/faccessat2/faccessat201.c
index bbe441b51..1be6e9aa0 100644
--- a/testcases/kernel/syscalls/faccessat2/faccessat201.c
+++ b/testcases/kernel/syscalls/faccessat2/faccessat201.c
@@ -56,7 +56,7 @@ static void verify_faccessat2(unsigned int i)
static void setup(void)
{
- abs_path = tst_tmpdir_mkpath(RELPATH);
+ abs_path = tst_tmpdir_genpath(RELPATH);
SAFE_MKDIR(TESTDIR, 0777);
dir_fd = SAFE_OPEN(TESTDIR, O_DIRECTORY);
diff --git a/testcases/kernel/syscalls/fchmodat/fchmodat01.c b/testcases/kernel/syscalls/fchmodat/fchmodat01.c
index 97ba31763..7ca65648f 100644
--- a/testcases/kernel/syscalls/fchmodat/fchmodat01.c
+++ b/testcases/kernel/syscalls/fchmodat/fchmodat01.c
@@ -60,7 +60,7 @@ static void verify_fchmodat(unsigned int i)
static void setup(void)
{
- abs_path = tst_tmpdir_mkpath(FILEPATH);
+ abs_path = tst_tmpdir_genpath(FILEPATH);
SAFE_MKDIR(TESTDIR, 0700);
dir_fd = SAFE_OPEN(TESTDIR, O_DIRECTORY);
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
index 2f0df3b27..e7b337e4a 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop01.c
@@ -126,7 +126,7 @@ static void setup(void)
sprintf(autoclear_path, "/sys/block/loop%d/loop/autoclear", dev_num);
sprintf(backing_path, "/sys/block/loop%d/loop/backing_file", dev_num);
sprintf(sys_loop_partpath, "/sys/block/loop%d/loop%dp1", dev_num, dev_num);
- backing_file_path = tst_tmpdir_mkpath("test.img");
+ backing_file_path = tst_tmpdir_genpath("test.img");
sprintf(loop_partpath, "%sp1", dev_path);
dev_fd = SAFE_OPEN(dev_path, O_RDWR);
}
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop02.c b/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
index fa193924a..dc983ac5f 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
@@ -121,8 +121,8 @@ static void setup(void)
tst_fill_file("test2.img", 0, 2048, 20);
sprintf(backing_path, "/sys/block/loop%d/loop/backing_file", dev_num);
- backing_file_path = tst_tmpdir_mkpath("test.img");
- backing_file_change_path = tst_tmpdir_mkpath("test1.img");
+ backing_file_path = tst_tmpdir_genpath("test.img");
+ backing_file_change_path = tst_tmpdir_genpath("test1.img");
sprintf(loop_ro_path, "/sys/block/loop%d/ro", dev_num);
file_change_fd = SAFE_OPEN("test1.img", O_RDWR);
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
index 6f19280cc..184462464 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
@@ -125,7 +125,7 @@ static void setup(void)
* size of loop is bigger than the backing device's and the loop
* needn't transform transfer.
*/
- backing_file_path = tst_tmpdir_mkpath("test.img");
+ backing_file_path = tst_tmpdir_genpath("test.img");
tst_find_backing_dev(backing_file_path, bd_path, sizeof(bd_path));
block_devfd = SAFE_OPEN(bd_path, O_RDWR);
SAFE_IOCTL(block_devfd, BLKSSZGET, &logical_block_size);
diff --git a/testcases/kernel/syscalls/mount/mount06.c b/testcases/kernel/syscalls/mount/mount06.c
index 8028dc5ec..05d6fdf86 100644
--- a/testcases/kernel/syscalls/mount/mount06.c
+++ b/testcases/kernel/syscalls/mount/mount06.c
@@ -43,10 +43,10 @@ static void setup(void)
SAFE_MOUNT(tmppath, tmppath, "none", MS_BIND, NULL);
SAFE_MOUNT("none", tmppath, "none", MS_PRIVATE, NULL);
- mntpoint_src = tst_tmpdir_mkpath(MNTPOINT_SRC);
- mntpoint_dst = tst_tmpdir_mkpath(MNTPOINT_DST);
- tstfiles_src = tst_tmpdir_mkpath("%s/testfile", MNTPOINT_SRC);
- tstfiles_dst = tst_tmpdir_mkpath("%s/testfile", MNTPOINT_DST);
+ mntpoint_src = tst_tmpdir_genpath(MNTPOINT_SRC);
+ mntpoint_dst = tst_tmpdir_genpath(MNTPOINT_DST);
+ tstfiles_src = tst_tmpdir_genpath("%s/testfile", MNTPOINT_SRC);
+ tstfiles_dst = tst_tmpdir_genpath("%s/testfile", MNTPOINT_DST);
SAFE_MKDIR(mntpoint_dst, 0750);
}
diff --git a/testcases/kernel/syscalls/mount/mount07.c b/testcases/kernel/syscalls/mount/mount07.c
index 495777067..8994d0f34 100644
--- a/testcases/kernel/syscalls/mount/mount07.c
+++ b/testcases/kernel/syscalls/mount/mount07.c
@@ -114,9 +114,9 @@ static void test_statfs(bool nosymfollow)
static void setup(void)
{
- test_file = tst_tmpdir_mkpath("%s/test_file", MNTPOINT);
- link_file = tst_tmpdir_mkpath("%s/link_file", MNTPOINT);
- temp_link_file = tst_tmpdir_mkpath("%s/temp_link_file", MNTPOINT);
+ test_file = tst_tmpdir_genpath("%s/test_file", MNTPOINT);
+ link_file = tst_tmpdir_genpath("%s/link_file", MNTPOINT);
+ temp_link_file = tst_tmpdir_genpath("%s/temp_link_file", MNTPOINT);
}
static void cleanup(void)
diff --git a/testcases/kernel/syscalls/pathconf/pathconf02.c b/testcases/kernel/syscalls/pathconf/pathconf02.c
index 42b97dc93..a7af980ec 100644
--- a/testcases/kernel/syscalls/pathconf/pathconf02.c
+++ b/testcases/kernel/syscalls/pathconf/pathconf02.c
@@ -70,7 +70,7 @@ static void setup(void)
SAFE_TOUCH("testfile", 0777, NULL);
- abs_path = tst_tmpdir_mkpath(FILEPATH);
+ abs_path = tst_tmpdir_genpath(FILEPATH);
SAFE_CHMOD(tst_tmpdir_path(), 0);
diff --git a/testcases/kernel/syscalls/readlinkat/readlinkat01.c b/testcases/kernel/syscalls/readlinkat/readlinkat01.c
index cd78ba134..9f238ff0a 100644
--- a/testcases/kernel/syscalls/readlinkat/readlinkat01.c
+++ b/testcases/kernel/syscalls/readlinkat/readlinkat01.c
@@ -66,7 +66,7 @@ static void verify_readlinkat(unsigned int i)
static void setup(void)
{
- abspath = tst_tmpdir_mkpath(TEST_SYMLINK);
+ abspath = tst_tmpdir_genpath(TEST_SYMLINK);
file_fd = SAFE_OPEN(TEST_FILE, O_CREAT, 0600);
SAFE_SYMLINK(TEST_FILE, TEST_SYMLINK);
diff --git a/testcases/kernel/syscalls/stat/stat04.c b/testcases/kernel/syscalls/stat/stat04.c
index 04ddcd2d1..05ace606a 100644
--- a/testcases/kernel/syscalls/stat/stat04.c
+++ b/testcases/kernel/syscalls/stat/stat04.c
@@ -49,8 +49,8 @@ static void setup(void)
int pagesize;
int fd;
- file_path = tst_tmpdir_mkpath(FILENAME);
- symb_path = tst_tmpdir_mkpath(SYMBNAME);
+ file_path = tst_tmpdir_genpath(FILENAME);
+ symb_path = tst_tmpdir_genpath(SYMBNAME);
/* change st_blksize / st_dev */
SAFE_STAT(".", &sb);
--
2.46.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] tmpdir: rename tst_tmpdir_mkpath to tst_tmpdir_genpath
2024-09-10 7:10 [LTP] [PATCH] tmpdir: rename tst_tmpdir_mkpath to tst_tmpdir_genpath Li Wang
@ 2024-09-10 8:52 ` Cyril Hrubis
2024-09-11 6:02 ` Li Wang
0 siblings, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2024-09-10 8:52 UTC (permalink / raw)
To: Li Wang; +Cc: ltp
Hi!
> When I reviewed the rename15 patch I felt confused a while on
> function like tst_tmpdir_mkpath (reminds me of: mkdir).
>
> Because the name could be misleading since it suggests that a
> file or directory is being created, when in fact it is simply
> constructing a path inside a temporary directory without
> actually creating any files.
Good catch, this is indeed confusing.
> To make the function's purpose clearer, the name should reflect
> that it is only constructing or generating a path, not creating
> any files or directories.
>
> So I think either 'tst_tmpdir_genpath' or 'tst_tmpdir_buildpath'
> would be concise and clear options.
Maybe tst_tmpdir_getpath()?
But I agree that any of these three is better than the original. Naming
API functions is hard...
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] tmpdir: rename tst_tmpdir_mkpath to tst_tmpdir_genpath
2024-09-10 8:52 ` Cyril Hrubis
@ 2024-09-11 6:02 ` Li Wang
2024-09-11 6:23 ` Jan Stancek
2024-09-11 15:00 ` Cyril Hrubis
0 siblings, 2 replies; 6+ messages in thread
From: Li Wang @ 2024-09-11 6:02 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
On Tue, Sep 10, 2024 at 4:53 PM Cyril Hrubis <chrubis@suse.cz> wrote:
> Hi!
> > When I reviewed the rename15 patch I felt confused a while on
> > function like tst_tmpdir_mkpath (reminds me of: mkdir).
> >
> > Because the name could be misleading since it suggests that a
> > file or directory is being created, when in fact it is simply
> > constructing a path inside a temporary directory without
> > actually creating any files.
>
> Good catch, this is indeed confusing.
>
> > To make the function's purpose clearer, the name should reflect
> > that it is only constructing or generating a path, not creating
> > any files or directories.
> >
> > So I think either 'tst_tmpdir_genpath' or 'tst_tmpdir_buildpath'
> > would be concise and clear options.
>
> Maybe tst_tmpdir_getpath()?
>
Hmm, I feel 'gen' is better than 'get', because get looks like there already
exist and we just get the path in a pointer. 'gen' is more like generating
a string but not creating it.
Maybe others have different opinions.
--
Regards,
Li Wang
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] tmpdir: rename tst_tmpdir_mkpath to tst_tmpdir_genpath
2024-09-11 6:02 ` Li Wang
@ 2024-09-11 6:23 ` Jan Stancek
2024-09-11 15:00 ` Cyril Hrubis
1 sibling, 0 replies; 6+ messages in thread
From: Jan Stancek @ 2024-09-11 6:23 UTC (permalink / raw)
To: Li Wang; +Cc: ltp
On Wed, Sep 11, 2024 at 8:02 AM Li Wang <liwang@redhat.com> wrote:
>
>
>
> On Tue, Sep 10, 2024 at 4:53 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>>
>> Hi!
>> > When I reviewed the rename15 patch I felt confused a while on
>> > function like tst_tmpdir_mkpath (reminds me of: mkdir).
>> >
>> > Because the name could be misleading since it suggests that a
>> > file or directory is being created, when in fact it is simply
>> > constructing a path inside a temporary directory without
>> > actually creating any files.
>>
>> Good catch, this is indeed confusing.
>>
>> > To make the function's purpose clearer, the name should reflect
>> > that it is only constructing or generating a path, not creating
>> > any files or directories.
>> >
>> > So I think either 'tst_tmpdir_genpath' or 'tst_tmpdir_buildpath'
>> > would be concise and clear options.
>>
>> Maybe tst_tmpdir_getpath()?
>
>
> Hmm, I feel 'gen' is better than 'get', because get looks like there already
> exist and we just get the path in a pointer. 'gen' is more like generating
> a string but not creating it.
>
> Maybe others have different opinions.
tst_tmpdir_pathfmt
tst_tmpdir_mkpathname - seems too long
given the choices above, genpath seems OK to me as well
>
>
> --
> Regards,
> Li Wang
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] tmpdir: rename tst_tmpdir_mkpath to tst_tmpdir_genpath
2024-09-11 6:02 ` Li Wang
2024-09-11 6:23 ` Jan Stancek
@ 2024-09-11 15:00 ` Cyril Hrubis
2024-09-12 0:46 ` Li Wang
1 sibling, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2024-09-11 15:00 UTC (permalink / raw)
To: Li Wang; +Cc: ltp
Hi!
> Hmm, I feel 'gen' is better than 'get', because get looks like there already
> exist and we just get the path in a pointer. 'gen' is more like generating
> a string but not creating it.
Feel free to go with 'gen' it's way better than the original.
Acked-by: Cyril Hrubis <chrubis@suse.cz>
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] tmpdir: rename tst_tmpdir_mkpath to tst_tmpdir_genpath
2024-09-11 15:00 ` Cyril Hrubis
@ 2024-09-12 0:46 ` Li Wang
0 siblings, 0 replies; 6+ messages in thread
From: Li Wang @ 2024-09-12 0:46 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
On Wed, Sep 11, 2024 at 11:01 PM Cyril Hrubis <chrubis@suse.cz> wrote:
> Hi!
> > Hmm, I feel 'gen' is better than 'get', because get looks like there
> already
> > exist and we just get the path in a pointer. 'gen' is more like
> generating
> > a string but not creating it.
>
> Feel free to go with 'gen' it's way better than the original.
>
> Acked-by: Cyril Hrubis <chrubis@suse.cz>
>
Pushed, thank you all.
--
Regards,
Li Wang
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-12 0:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-10 7:10 [LTP] [PATCH] tmpdir: rename tst_tmpdir_mkpath to tst_tmpdir_genpath Li Wang
2024-09-10 8:52 ` Cyril Hrubis
2024-09-11 6:02 ` Li Wang
2024-09-11 6:23 ` Jan Stancek
2024-09-11 15:00 ` Cyril Hrubis
2024-09-12 0:46 ` Li Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox