diff mbox series

[v1] Add readlink04 test

Message ID 20240118150742.10792-1-andrea.cervesato@suse.de
State Rejected
Headers show
Series [v1] Add readlink04 test | expand

Commit Message

Andrea Cervesato Jan. 18, 2024, 3:07 p.m. UTC 
From: Andrea Cervesato <andrea.cervesato@suse.com>

This test has been extracted from symlink01 and it verifies that
readlink() is working correctly on symlink() generated files.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/syscalls                              |  2 +-
 testcases/kernel/syscalls/readlink/.gitignore |  1 +
 .../kernel/syscalls/readlink/readlink04.c     | 57 +++++++++++++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 testcases/kernel/syscalls/readlink/readlink04.c

Comments

Cyril Hrubis Feb. 2, 2024, 4:17 p.m. UTC | #1 
On Thu, Jan 18, 2024 at 04:07:42PM +0100, Andrea Cervesato wrote:
> From: Andrea Cervesato <andrea.cervesato@suse.com>
> 
> This test has been extracted from symlink01 and it verifies that
> readlink() is working correctly on symlink() generated files.
> 
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  runtest/syscalls                              |  2 +-
>  testcases/kernel/syscalls/readlink/.gitignore |  1 +
>  .../kernel/syscalls/readlink/readlink04.c     | 57 +++++++++++++++++++
>  3 files changed, 59 insertions(+), 1 deletion(-)
>  create mode 100644 testcases/kernel/syscalls/readlink/readlink04.c
> 
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 6e2407879..339697533 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1125,9 +1125,9 @@ readahead02 readahead02
>  readdir01 readdir01
>  readdir21 readdir21
>  
> -readlink01A symlink01 -T readlink01
>  readlink01 readlink01
>  readlink03 readlink03
> +readlink04 readlink04
>  
>  #readlinkat test cases
>  readlinkat01 readlinkat01
> diff --git a/testcases/kernel/syscalls/readlink/.gitignore b/testcases/kernel/syscalls/readlink/.gitignore
> index 307817f4d..53e65eb5b 100644
> --- a/testcases/kernel/syscalls/readlink/.gitignore
> +++ b/testcases/kernel/syscalls/readlink/.gitignore
> @@ -1,2 +1,3 @@
>  /readlink01
>  /readlink03
> +/readlink04
> diff --git a/testcases/kernel/syscalls/readlink/readlink04.c b/testcases/kernel/syscalls/readlink/readlink04.c
> new file mode 100644
> index 000000000..8d7d038bb
> --- /dev/null
> +++ b/testcases/kernel/syscalls/readlink/readlink04.c
> @@ -0,0 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
> + *    Author: David Fenner
> + *    Copilot: Jon Hendrickson
> + * Copyright (C) 2024 Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * This test verifies that readlink() is working correctly on symlink()
> + * generated files.
> + */
> +
> +#include "tst_test.h"
> +
> +static void test_readlink(void)
> +{
> +	char *symname = "my_symlink0";
> +
> +	SAFE_SYMLINK(tst_get_tmpdir(), symname);
> +
> +	char path[PATH_MAX];
> +	struct stat path_link;
> +
> +	SAFE_READLINK(symname, path, PATH_MAX);
> +	TST_EXP_PASS(lstat(path, &path_link));
> +
> +	SAFE_UNLINK(symname);
> +}

Isn't this already tested in readlink01?

And actually the readlink01 is doing this better by comparing the string
returned from readlink() against the one we passed to the sysmlink()
syscall previously. The lastat() check is indirectly asserting that we
got right value as well, however simple memcmp() is better.

> +static void test_readlink_no_path(void)
> +{
> +	char *symname = "my_symlink1";
> +
> +	SAFE_SYMLINK("bc+eFhi!k", symname);
> +
> +	char path[PATH_MAX];
> +	struct stat path_link;
> +
> +	SAFE_READLINK(symname, path, PATH_MAX);
> +	TST_EXP_FAIL(lstat(path, &path_link), ENOENT);
> +
> +	SAFE_UNLINK(symname);
> +}

Maybe it would be easier to add this to symlink01 as well?

In reality it probably does not make much difference if we readlink a
symlink to an existing or non-existing file though...
Andrea Cervesato Feb. 16, 2024, 1:29 p.m. UTC | #2 
Hi!

On 2/2/24 17:17, Cyril Hrubis wrote:
> On Thu, Jan 18, 2024 at 04:07:42PM +0100, Andrea Cervesato wrote:
>> From: Andrea Cervesato <andrea.cervesato@suse.com>
>>
>> This test has been extracted from symlink01 and it verifies that
>> readlink() is working correctly on symlink() generated files.
>>
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>>   runtest/syscalls                              |  2 +-
>>   testcases/kernel/syscalls/readlink/.gitignore |  1 +
>>   .../kernel/syscalls/readlink/readlink04.c     | 57 +++++++++++++++++++
>>   3 files changed, 59 insertions(+), 1 deletion(-)
>>   create mode 100644 testcases/kernel/syscalls/readlink/readlink04.c
>>
>> diff --git a/runtest/syscalls b/runtest/syscalls
>> index 6e2407879..339697533 100644
>> --- a/runtest/syscalls
>> +++ b/runtest/syscalls
>> @@ -1125,9 +1125,9 @@ readahead02 readahead02
>>   readdir01 readdir01
>>   readdir21 readdir21
>>   
>> -readlink01A symlink01 -T readlink01
>>   readlink01 readlink01
>>   readlink03 readlink03
>> +readlink04 readlink04
>>   
>>   #readlinkat test cases
>>   readlinkat01 readlinkat01
>> diff --git a/testcases/kernel/syscalls/readlink/.gitignore b/testcases/kernel/syscalls/readlink/.gitignore
>> index 307817f4d..53e65eb5b 100644
>> --- a/testcases/kernel/syscalls/readlink/.gitignore
>> +++ b/testcases/kernel/syscalls/readlink/.gitignore
>> @@ -1,2 +1,3 @@
>>   /readlink01
>>   /readlink03
>> +/readlink04
>> diff --git a/testcases/kernel/syscalls/readlink/readlink04.c b/testcases/kernel/syscalls/readlink/readlink04.c
>> new file mode 100644
>> index 000000000..8d7d038bb
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/readlink/readlink04.c
>> @@ -0,0 +1,57 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
>> + *    Author: David Fenner
>> + *    Copilot: Jon Hendrickson
>> + * Copyright (C) 2024 Andrea Cervesato <andrea.cervesato@suse.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * This test verifies that readlink() is working correctly on symlink()
>> + * generated files.
>> + */
>> +
>> +#include "tst_test.h"
>> +
>> +static void test_readlink(void)
>> +{
>> +	char *symname = "my_symlink0";
>> +
>> +	SAFE_SYMLINK(tst_get_tmpdir(), symname);
>> +
>> +	char path[PATH_MAX];
>> +	struct stat path_link;
>> +
>> +	SAFE_READLINK(symname, path, PATH_MAX);
>> +	TST_EXP_PASS(lstat(path, &path_link));
>> +
>> +	SAFE_UNLINK(symname);
>> +}
> Isn't this already tested in readlink01?
>
> And actually the readlink01 is doing this better by comparing the string
> returned from readlink() against the one we passed to the sysmlink()
> syscall previously. The lastat() check is indirectly asserting that we
> got right value as well, however simple memcmp() is better.
>
I agree, please ignore this test. I think we can just skip it
>> +static void test_readlink_no_path(void)
>> +{
>> +	char *symname = "my_symlink1";
>> +
>> +	SAFE_SYMLINK("bc+eFhi!k", symname);
>> +
>> +	char path[PATH_MAX];
>> +	struct stat path_link;
>> +
>> +	SAFE_READLINK(symname, path, PATH_MAX);
>> +	TST_EXP_FAIL(lstat(path, &path_link), ENOENT);
>> +
>> +	SAFE_UNLINK(symname);
>> +}
> Maybe it would be easier to add this to symlink01 as well?
>
> In reality it probably does not make much difference if we readlink a
> symlink to an existing or non-existing file though...
>
Andrea
Andrea Cervesato Feb. 19, 2024, 2:39 p.m. UTC | #3 
Hi,

taking in consideration the comments, I think we can just forget about it.
This test doesn't add anything and we can skip this patch.

Andrea

On 2/2/24 17:17, Cyril Hrubis wrote:
> On Thu, Jan 18, 2024 at 04:07:42PM +0100, Andrea Cervesato wrote:
>> From: Andrea Cervesato <andrea.cervesato@suse.com>
>>
>> This test has been extracted from symlink01 and it verifies that
>> readlink() is working correctly on symlink() generated files.
>>
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>>   runtest/syscalls                              |  2 +-
>>   testcases/kernel/syscalls/readlink/.gitignore |  1 +
>>   .../kernel/syscalls/readlink/readlink04.c     | 57 +++++++++++++++++++
>>   3 files changed, 59 insertions(+), 1 deletion(-)
>>   create mode 100644 testcases/kernel/syscalls/readlink/readlink04.c
>>
>> diff --git a/runtest/syscalls b/runtest/syscalls
>> index 6e2407879..339697533 100644
>> --- a/runtest/syscalls
>> +++ b/runtest/syscalls
>> @@ -1125,9 +1125,9 @@ readahead02 readahead02
>>   readdir01 readdir01
>>   readdir21 readdir21
>>   
>> -readlink01A symlink01 -T readlink01
>>   readlink01 readlink01
>>   readlink03 readlink03
>> +readlink04 readlink04
>>   
>>   #readlinkat test cases
>>   readlinkat01 readlinkat01
>> diff --git a/testcases/kernel/syscalls/readlink/.gitignore b/testcases/kernel/syscalls/readlink/.gitignore
>> index 307817f4d..53e65eb5b 100644
>> --- a/testcases/kernel/syscalls/readlink/.gitignore
>> +++ b/testcases/kernel/syscalls/readlink/.gitignore
>> @@ -1,2 +1,3 @@
>>   /readlink01
>>   /readlink03
>> +/readlink04
>> diff --git a/testcases/kernel/syscalls/readlink/readlink04.c b/testcases/kernel/syscalls/readlink/readlink04.c
>> new file mode 100644
>> index 000000000..8d7d038bb
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/readlink/readlink04.c
>> @@ -0,0 +1,57 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
>> + *    Author: David Fenner
>> + *    Copilot: Jon Hendrickson
>> + * Copyright (C) 2024 Andrea Cervesato <andrea.cervesato@suse.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * This test verifies that readlink() is working correctly on symlink()
>> + * generated files.
>> + */
>> +
>> +#include "tst_test.h"
>> +
>> +static void test_readlink(void)
>> +{
>> +	char *symname = "my_symlink0";
>> +
>> +	SAFE_SYMLINK(tst_get_tmpdir(), symname);
>> +
>> +	char path[PATH_MAX];
>> +	struct stat path_link;
>> +
>> +	SAFE_READLINK(symname, path, PATH_MAX);
>> +	TST_EXP_PASS(lstat(path, &path_link));
>> +
>> +	SAFE_UNLINK(symname);
>> +}
> Isn't this already tested in readlink01?
>
> And actually the readlink01 is doing this better by comparing the string
> returned from readlink() against the one we passed to the sysmlink()
> syscall previously. The lastat() check is indirectly asserting that we
> got right value as well, however simple memcmp() is better.
>
>> +static void test_readlink_no_path(void)
>> +{
>> +	char *symname = "my_symlink1";
>> +
>> +	SAFE_SYMLINK("bc+eFhi!k", symname);
>> +
>> +	char path[PATH_MAX];
>> +	struct stat path_link;
>> +
>> +	SAFE_READLINK(symname, path, PATH_MAX);
>> +	TST_EXP_FAIL(lstat(path, &path_link), ENOENT);
>> +
>> +	SAFE_UNLINK(symname);
>> +}
> Maybe it would be easier to add this to symlink01 as well?
>
> In reality it probably does not make much difference if we readlink a
> symlink to an existing or non-existing file though...
>
diff mbox series

Patch

diff --git a/runtest/syscalls b/runtest/syscalls
index 6e2407879..339697533 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1125,9 +1125,9 @@  readahead02 readahead02
 readdir01 readdir01
 readdir21 readdir21
 
-readlink01A symlink01 -T readlink01
 readlink01 readlink01
 readlink03 readlink03
+readlink04 readlink04
 
 #readlinkat test cases
 readlinkat01 readlinkat01
diff --git a/testcases/kernel/syscalls/readlink/.gitignore b/testcases/kernel/syscalls/readlink/.gitignore
index 307817f4d..53e65eb5b 100644
--- a/testcases/kernel/syscalls/readlink/.gitignore
+++ b/testcases/kernel/syscalls/readlink/.gitignore
@@ -1,2 +1,3 @@ 
 /readlink01
 /readlink03
+/readlink04
diff --git a/testcases/kernel/syscalls/readlink/readlink04.c b/testcases/kernel/syscalls/readlink/readlink04.c
new file mode 100644
index 000000000..8d7d038bb
--- /dev/null
+++ b/testcases/kernel/syscalls/readlink/readlink04.c
@@ -0,0 +1,57 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2000 Silicon Graphics, Inc.  All Rights Reserved.
+ *    Author: David Fenner
+ *    Copilot: Jon Hendrickson
+ * Copyright (C) 2024 Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * This test verifies that readlink() is working correctly on symlink()
+ * generated files.
+ */
+
+#include "tst_test.h"
+
+static void test_readlink(void)
+{
+	char *symname = "my_symlink0";
+
+	SAFE_SYMLINK(tst_get_tmpdir(), symname);
+
+	char path[PATH_MAX];
+	struct stat path_link;
+
+	SAFE_READLINK(symname, path, PATH_MAX);
+	TST_EXP_PASS(lstat(path, &path_link));
+
+	SAFE_UNLINK(symname);
+}
+
+static void test_readlink_no_path(void)
+{
+	char *symname = "my_symlink1";
+
+	SAFE_SYMLINK("bc+eFhi!k", symname);
+
+	char path[PATH_MAX];
+	struct stat path_link;
+
+	SAFE_READLINK(symname, path, PATH_MAX);
+	TST_EXP_FAIL(lstat(path, &path_link), ENOENT);
+
+	SAFE_UNLINK(symname);
+}
+
+static void run(void)
+{
+	test_readlink();
+	test_readlink_no_path();
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.needs_tmpdir = 1,
+};