[LTP] [PATCH 1/2] lgetxattr/lgetxattr01.c: add new testcase

[LTP] [PATCH 1/2] lgetxattr/lgetxattr01.c: add new testcase

[Huang Jin Bao]

The testcase checks the basic functionality of the lgetxattr(2).
In the case of a symbolic link, we only get the value of the
extended attribute related to the link itself by name.

Signed-off-by: Huang Jin Bao <huangjb.jy@cn.fujitsu.com>
---
 runtest/ltplite                                   |   2 +
 runtest/syscalls                                  |   2 +
 testcases/kernel/syscalls/.gitignore              |   1 +
 testcases/kernel/syscalls/lgetxattr/Makefile      |  23 +++++
 testcases/kernel/syscalls/lgetxattr/lgetxattr01.c | 119 ++++++++++++++++++++++
 5 files changed, 147 insertions(+)
 create mode 100644 testcases/kernel/syscalls/lgetxattr/Makefile
 create mode 100644 testcases/kernel/syscalls/lgetxattr/lgetxattr01.c

diff --git a/runtest/ltplite b/runtest/ltplite
index 9e1f201..9d10af2 100644
--- a/runtest/ltplite
+++ b/runtest/ltplite
@@ -385,6 +385,8 @@ lchown01 lchown01
 lchown02 lchown02
 lchown03 lchown03
 
+lgetxattr01 lgetxattr01
+
 link01 symlink01 -T link01
 link02 link02
 link03 link03
diff --git a/runtest/syscalls b/runtest/syscalls
index 428b774..e66fa87 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -505,6 +505,8 @@ lchown03  lchown03
 lchown02_16 lchown02_16
 lchown03_16 lchown03_16
 
+lgetxattr01 lgetxattr01
+
 link01 symlink01 -T link01
 link02 link02
 link03 link03
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 81f4890..4b3aa9d 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -470,6 +470,7 @@
 /lchown/lchown02_16
 /lchown/lchown03
 /lchown/lchown03_16
+/lgetxattr/lgetxattr01
 /link/link02
 /link/link03
 /link/link04
diff --git a/testcases/kernel/syscalls/lgetxattr/Makefile b/testcases/kernel/syscalls/lgetxattr/Makefile
new file mode 100644
index 0000000..10d4499
--- /dev/null
+++ b/testcases/kernel/syscalls/lgetxattr/Makefile
@@ -0,0 +1,23 @@
+#
+#  Copyright (c) 2016 Fujitsu Ltd.
+#  Author: Jinbao Huang <huangjb.jy@cn.fujitsu.com>
+#
+#  This program is free software;  you can redistribute it and/or modify
+#  it under the terms of the GNU General Public License as published by
+#  the Free Software Foundation; either version 2 of the License, or
+#  (at your option) any later version.
+#
+#  This program is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY;  without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+#  the GNU General Public License for more details.
+#
+#  You should have received a copy of the GNU General Public License
+#  along with this program.
+#
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/lgetxattr/lgetxattr01.c b/testcases/kernel/syscalls/lgetxattr/lgetxattr01.c
new file mode 100644
index 0000000..d96ba8e
--- /dev/null
+++ b/testcases/kernel/syscalls/lgetxattr/lgetxattr01.c
@@ -0,0 +1,119 @@
+/*
+* Copyright (c) 2016 Fujitsu Ltd.
+* Author: Jinbao Huang <huangjb.jy@cn.fujitsu.com>
+*
+* This program is free software; you can redistribute it and/or modify it
+* under the terms of version 2 of the GNU General Public License as
+* published by the Free Software Foundation.
+*
+* This program is distributed in the hope that it would be useful, but
+* WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+*
+* You should have received a copy of the GNU General Public License
+* alone with this program.
+*/
+
+/*
+* Test Name: lgetxattr01
+*
+* Description:
+* The testcase checks the basic functionality of the lgetxattr(2).
+* In the case of a symbolic link, we only get the value of the
+* extended attribute related to the link itself by name.
+*/
+
+#include "config.h"
+#include <errno.h>
+#include <sys/types.h>
+#include <string.h>
+
+#ifdef HAVE_ATTR_XATTR_H
+#include <attr/xattr.h>
+#endif
+
+#include "tst_test.h"
+
+#ifdef HAVE_ATTR_XATTR_H
+#define SECURITY_KEY1   "security.ltptest1"
+#define SECURITY_KEY2   "security.ltptest2"
+#define VALUE1   "test1"
+#define VALUE2   "test2"
+#define VALUE_SIZE      5
+
+static void set_xattr(char *path, char *key, void *value)
+{
+	int res;
+
+	res = lsetxattr(path, key, value, VALUE_SIZE, XATTR_CREATE);
+	if (res == -1) {
+		if (errno == ENOTSUP) {
+			tst_brk(TCONF,
+				"no xattr support in fs or mounted "
+				"without user_xattr option");
+		}
+
+		if (errno == EEXIST)
+			tst_brk(TBROK, "exist attribute %s", key);
+		else
+			tst_brk(TBROK | TERRNO, "lsetxattr() failed");
+	}
+}
+
+static void setup(void)
+{
+	SAFE_TOUCH("testfile", 0644, NULL);
+	SAFE_SYMLINK("testfile", "symlink");
+
+	set_xattr("testfile", SECURITY_KEY1, VALUE1);
+	set_xattr("symlink", SECURITY_KEY2, VALUE2);
+}
+
+static void verify_lgetxattr(void)
+{
+	int size = 64;
+	char buf[size];
+
+	TEST(lgetxattr("symlink", SECURITY_KEY2, buf, size));
+	if (TEST_RETURN == -1) {
+		tst_res(TFAIL | TTERRNO, "lgetxattr() failed");
+		return;
+	}
+
+	if (TEST_RETURN != VALUE_SIZE) {
+		tst_res(TFAIL, "lgetxattr() got unexpected value size");
+		return;
+	}
+
+	if (!strncmp(buf, VALUE2, TEST_RETURN))
+		tst_res(TPASS, "lgetxattr() get expect value");
+	else
+		tst_res(TFAIL, "lgetxattr() got unexpected value");
+
+	TEST(lgetxattr("symlink", SECURITY_KEY1, buf, size));
+
+	if (TEST_RETURN != -1) {
+		tst_res(TFAIL, "lgetxattr() succeeded unexpectedly");
+		return;
+	}
+
+	if (TEST_ERRNO == ENODATA) {
+		tst_res(TPASS | TTERRNO, "lgetxattr() failed as expected");
+	} else {
+		tst_res(TFAIL | TTERRNO,
+			"lgetxattr() failed unexpectedly,"
+			"expected %s", tst_strerrno(ENODATA));
+	}
+}
+
+static struct tst_test test = {
+	.tid = "lgetxattr01",
+	.needs_tmpdir = 1,
+	.needs_root = 1,
+	.test_all = verify_lgetxattr,
+	.setup = setup,
+};
+
+#else
+	TST_TEST_TCONF("<attr/xattr.h> does not exist.");
+#endif /* HAVE_ATTR_XATTR_H */
-- 
1.8.3.1

[LTP] [PATCH 2/2] lgetxattr/lgetxattr02.c: add new testcase

[Huang Jin Bao]

1) lgetxattr(2) fails if the named attribute does not exist
   or the process has no access to this attribute.
2) lgetxattr(2) fails if the size of the value buffer is too small
   to hold the result.
3) lgetxattr(2) fails when attemptes to read from a invalid address.

Signed-off-by: Huang Jin Bao <huangjb.jy@cn.fujitsu.com>
---
 runtest/ltplite                                   |   1 +
 runtest/syscalls                                  |   1 +
 testcases/kernel/syscalls/.gitignore              |   1 +
 testcases/kernel/syscalls/lgetxattr/lgetxattr02.c | 110 ++++++++++++++++++++++
 4 files changed, 113 insertions(+)
 create mode 100644 testcases/kernel/syscalls/lgetxattr/lgetxattr02.c

diff --git a/runtest/ltplite b/runtest/ltplite
index 9d10af2..a08e147 100644
--- a/runtest/ltplite
+++ b/runtest/ltplite
@@ -386,6 +386,7 @@ lchown02 lchown02
 lchown03 lchown03
 
 lgetxattr01 lgetxattr01
+lgetxattr02 lgetxattr02
 
 link01 symlink01 -T link01
 link02 link02
diff --git a/runtest/syscalls b/runtest/syscalls
index e66fa87..a838a5d 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -506,6 +506,7 @@ lchown02_16 lchown02_16
 lchown03_16 lchown03_16
 
 lgetxattr01 lgetxattr01
+lgetxattr02 lgetxattr02
 
 link01 symlink01 -T link01
 link02 link02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 4b3aa9d..5474795 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -471,6 +471,7 @@
 /lchown/lchown03
 /lchown/lchown03_16
 /lgetxattr/lgetxattr01
+/lgetxattr/lgetxattr02
 /link/link02
 /link/link03
 /link/link04
diff --git a/testcases/kernel/syscalls/lgetxattr/lgetxattr02.c b/testcases/kernel/syscalls/lgetxattr/lgetxattr02.c
new file mode 100644
index 0000000..c911658
--- /dev/null
+++ b/testcases/kernel/syscalls/lgetxattr/lgetxattr02.c
@@ -0,0 +1,110 @@
+/*
+* Copyright (c) 2016 Fujitsu Ltd.
+* Author: Jinbao Huang <huangjb.jy@cn.fujitsu.com>
+*
+* This program is free software; you can redistribute it and/or modify it
+* under the terms of version 2 of the GNU General Public License as
+* published by the Free Software Foundation.
+*
+* This program is distributed in the hope that it would be useful, but
+* WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+*
+* You should have received a copy of the GNU General Public License
+* alone with this program.
+*
+*/
+
+/*
+* Test Name: lgetxattr02
+*
+* Description:
+* 1) lgetxattr(2) fails if the named attribute does not exist
+*    or the process has no access to this attribute.
+* 2) lgetxattr(2) fails if the size of the value buffer is too small
+*    to hold the result.
+* 3) lgetxattr(2) fails when attemptes to read from a invalid address.
+*
+* Expected Result:
+* 1) lgetxattr(2) should return -1 and set errno to ENOATTR.
+* 2) lgetxattr(2) should return -1 and set errno to ERANGE.
+* 3) lgetxattr(2) should return -1 and set errno to EFAULT.
+*/
+
+#include "config.h"
+#include <errno.h>
+#include <sys/types.h>
+
+#ifdef HAVE_ATTR_XATTR_H
+#include <attr/xattr.h>
+#endif
+
+#include "tst_test.h"
+
+#ifdef HAVE_ATTR_XATTR_H
+
+#define SECURITY_KEY	"security.ltptest"
+#define VALUE	"this is a test value"
+#define VALUE_SIZE	20
+
+static struct test_case {
+	const char *path;
+	size_t size;
+	int exp_err;
+} tc[] = {
+	{"testfile", 19, ENOATTR},
+	{"symlink", 1, ERANGE},
+	{(char *)-1, 20, EFAULT}
+};
+
+static void verify_lgetxattr(unsigned int n)
+{
+	struct test_case *t = tc + n;
+	char buf[t->size];
+
+	TEST(lgetxattr(t->path, SECURITY_KEY, buf, tc->size));
+	if (TEST_RETURN != -1) {
+		tst_res(TFAIL, "lgetxattr() succeeded unexpectedly");
+	} else {
+		if (TEST_ERRNO != t->exp_err) {
+			tst_res(TFAIL | TTERRNO, "lgetxattr() failed "
+				"unexpectedlly, expected %s",
+				tst_strerrno(t->exp_err));
+		} else {
+			tst_res(TPASS | TTERRNO,
+				"lgetxattr() failed as expected");
+		}
+	}
+}
+
+static void setup(void)
+{
+	int n;
+
+	SAFE_TOUCH("testfile", 0644, NULL);
+
+	SAFE_SYMLINK("testfile", "symlink");
+
+	n = lsetxattr("symlink", SECURITY_KEY, VALUE, VALUE_SIZE, XATTR_CREATE);
+	if (n == -1) {
+		if (errno == ENOTSUP) {
+			tst_brk(TCONF, "no xattr support in fs or "
+				"mounted without user_xattr option");
+		} else {
+			tst_brk(TBROK | TERRNO, "lsetxattr() failed");
+		}
+	}
+}
+
+static struct tst_test test = {
+	.tid = "lgetxattr02",
+	.needs_tmpdir = 1,
+	.needs_root = 1,
+	.test = verify_lgetxattr,
+	.tcnt = ARRAY_SIZE(tc),
+	.setup = setup,
+};
+
+#else /* HAVE_ATTR_XATTR_H */
+	TST_TEST_TCONF("<attr/xattr.h> does not exist.");
+#endif
-- 
1.8.3.1

[LTP] [PATCH 1/2] lgetxattr/lgetxattr01.c: add new testcase

[Cyril Hrubis]

Hi!
> @@ -0,0 +1,119 @@
> +/*
> +* Copyright (c) 2016 Fujitsu Ltd.
> +* Author: Jinbao Huang <huangjb.jy@cn.fujitsu.com>
> +*
> +* This program is free software; you can redistribute it and/or modify it
> +* under the terms of version 2 of the GNU General Public License as
> +* published by the Free Software Foundation.
> +*
> +* This program is distributed in the hope that it would be useful, but
> +* WITHOUT ANY WARRANTY; without even the implied warranty of
> +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> +*
> +* You should have received a copy of the GNU General Public License
> +* alone with this program.
> +*/
> +
> +/*
> +* Test Name: lgetxattr01
> +*
> +* Description:
> +* The testcase checks the basic functionality of the lgetxattr(2).
> +* In the case of a symbolic link, we only get the value of the
> +* extended attribute related to the link itself by name.
> +*/
> +
> +#include "config.h"
> +#include <errno.h>
> +#include <sys/types.h>
> +#include <string.h>
> +
> +#ifdef HAVE_ATTR_XATTR_H
> +#include <attr/xattr.h>
> +#endif
> +
> +#include "tst_test.h"
> +
> +#ifdef HAVE_ATTR_XATTR_H
> +#define SECURITY_KEY1   "security.ltptest1"
> +#define SECURITY_KEY2   "security.ltptest2"
> +#define VALUE1   "test1"
> +#define VALUE2   "test2"
> +#define VALUE_SIZE      5
> +
> +static void set_xattr(char *path, char *key, void *value)
> +{
> +	int res;
> +
> +	res = lsetxattr(path, key, value, VALUE_SIZE, XATTR_CREATE);
                                           ^
                                          Should be either defined to
					  sizeof() or we can do strlen()
					  on the value as well.


I would either add size parameter to this function so that it could be
called in setup as:

set_xattr("testfile", SECURITY_KEY1, VALUE1, sizeof(VALUE1));

Which will include the terminating null character at the end, but I
doubt that we care about if it's there or not.

Or we can alternatively get the size with strlen(value) since it's a
string.

> +	if (res == -1) {
> +		if (errno == ENOTSUP) {
> +			tst_brk(TCONF,
> +				"no xattr support in fs or mounted "
> +				"without user_xattr option");
> +		}
> +
> +		if (errno == EEXIST)
> +			tst_brk(TBROK, "exist attribute %s", key);
> +		else
> +			tst_brk(TBROK | TERRNO, "lsetxattr() failed");

Why do we have special branch for the EEXIST case here?

We can just instead do:

	tst_brk(TBROK | TERRNO, "lsetxattr(%s)", key);

> +	}
> +}
> +
> +static void setup(void)
> +{
> +	SAFE_TOUCH("testfile", 0644, NULL);
> +	SAFE_SYMLINK("testfile", "symlink");
> +
> +	set_xattr("testfile", SECURITY_KEY1, VALUE1);
> +	set_xattr("symlink", SECURITY_KEY2, VALUE2);
> +}
> +
> +static void verify_lgetxattr(void)
> +{
> +	int size = 64;
> +	char buf[size];
> +
> +	TEST(lgetxattr("symlink", SECURITY_KEY2, buf, size));
> +	if (TEST_RETURN == -1) {
> +		tst_res(TFAIL | TTERRNO, "lgetxattr() failed");
> +		return;
> +	}
> +
> +	if (TEST_RETURN != VALUE_SIZE) {
> +		tst_res(TFAIL, "lgetxattr() got unexpected value size");

We may as well try to print the buffer here somehow, possibly as
hexadecimal data, since when we fail we will get pretty much random data
. Unfortunately the tst_resm_hexd() is not implemented in the new
library yet.

> +		return;
> +	}

Well we can possibly do goto next; here instead of the return so that we
do not skipp the next test if the first one fails, but I'm fine with
code as it is as well.

> +	if (!strncmp(buf, VALUE2, TEST_RETURN))
> +		tst_res(TPASS, "lgetxattr() get expect value");
> +	else
> +		tst_res(TFAIL, "lgetxattr() got unexpected value");
> +
> +	TEST(lgetxattr("symlink", SECURITY_KEY1, buf, size));
> +
> +	if (TEST_RETURN != -1) {
> +		tst_res(TFAIL, "lgetxattr() succeeded unexpectedly");
> +		return;
> +	}
> +
> +	if (TEST_ERRNO == ENODATA) {
> +		tst_res(TPASS | TTERRNO, "lgetxattr() failed as expected");
> +	} else {
> +		tst_res(TFAIL | TTERRNO,
> +			"lgetxattr() failed unexpectedly,"
> +			"expected %s", tst_strerrno(ENODATA));
> +	}
> +}
> +
> +static struct tst_test test = {
> +	.tid = "lgetxattr01",
> +	.needs_tmpdir = 1,
> +	.needs_root = 1,
> +	.test_all = verify_lgetxattr,
> +	.setup = setup,
> +};
> +
> +#else
> +	TST_TEST_TCONF("<attr/xattr.h> does not exist.");
> +#endif /* HAVE_ATTR_XATTR_H */

Otherwise this looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/2] lgetxattr/lgetxattr02.c: add new testcase

[Cyril Hrubis]

Hi!
> +* Description:
> +* 1) lgetxattr(2) fails if the named attribute does not exist
> +*    or the process has no access to this attribute.
> +* 2) lgetxattr(2) fails if the size of the value buffer is too small
> +*    to hold the result.
> +* 3) lgetxattr(2) fails when attemptes to read from a invalid address.
> +*
> +* Expected Result:
> +* 1) lgetxattr(2) should return -1 and set errno to ENOATTR.
> +* 2) lgetxattr(2) should return -1 and set errno to ERANGE.
> +* 3) lgetxattr(2) should return -1 and set errno to EFAULT.
> +*/
> +
> +#include "config.h"
> +#include <errno.h>
> +#include <sys/types.h>
> +
> +#ifdef HAVE_ATTR_XATTR_H
> +#include <attr/xattr.h>
> +#endif
> +
> +#include "tst_test.h"
> +
> +#ifdef HAVE_ATTR_XATTR_H
> +
> +#define SECURITY_KEY	"security.ltptest"
> +#define VALUE	"this is a test value"
> +#define VALUE_SIZE	20

Here again, define the VALUE_SIZE to sizeof(VALUE) or (sizeof(VALUE)-1)
if you want to avoid the terminating null character.

> +static struct test_case {
> +	const char *path;
> +	size_t size;
> +	int exp_err;
> +} tc[] = {
> +	{"testfile", 19, ENOATTR},
                      ^
		      Shouldn't this be VALUE_SIZE?
> +	{"symlink", 1, ERANGE},
> +	{(char *)-1, 20, EFAULT}
                     ^
                     And this as well?
> +};
> +
> +static void verify_lgetxattr(unsigned int n)
> +{
> +	struct test_case *t = tc + n;
> +	char buf[t->size];
> +
> +	TEST(lgetxattr(t->path, SECURITY_KEY, buf, tc->size));
                                                   ^
						   t->size ?

						   or even better
						   sizeof(buf)
> +	if (TEST_RETURN != -1) {
> +		tst_res(TFAIL, "lgetxattr() succeeded unexpectedly");

Ff you do return here would be no need for else branch and the code
below would be intended only by one tab.

> +	} else {
> +		if (TEST_ERRNO != t->exp_err) {
> +			tst_res(TFAIL | TTERRNO, "lgetxattr() failed "
> +				"unexpectedlly, expected %s",
> +				tst_strerrno(t->exp_err));
> +		} else {
> +			tst_res(TPASS | TTERRNO,
> +				"lgetxattr() failed as expected");
> +		}
> +	}
> +}
> +
> +static void setup(void)
> +{
> +	int n;
> +
> +	SAFE_TOUCH("testfile", 0644, NULL);
> +
> +	SAFE_SYMLINK("testfile", "symlink");
> +
> +	n = lsetxattr("symlink", SECURITY_KEY, VALUE, VALUE_SIZE, XATTR_CREATE);
> +	if (n == -1) {
> +		if (errno == ENOTSUP) {
> +			tst_brk(TCONF, "no xattr support in fs or "
> +				"mounted without user_xattr option");
> +		} else {
> +			tst_brk(TBROK | TERRNO, "lsetxattr() failed");
> +		}
> +	}
> +}
> +
> +static struct tst_test test = {
> +	.tid = "lgetxattr02",
> +	.needs_tmpdir = 1,
> +	.needs_root = 1,
> +	.test = verify_lgetxattr,
> +	.tcnt = ARRAY_SIZE(tc),
> +	.setup = setup,
> +};
> +
> +#else /* HAVE_ATTR_XATTR_H */
> +	TST_TEST_TCONF("<attr/xattr.h> does not exist.");
> +#endif

Otherwise this looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 1/2] lgetxattr/lgetxattr01.c: add new testcase

[Xiao Yang]

On 2016/07/13 20:22, Cyril Hrubis wrote:
Hi Cyril

I'm afraid that huangjb.jy@cn.fujitsu.com no longer works as huang was 
here only for his internship.
Thanks for your review, and i will send these patches v2.

Regards,
Xiao Yang
> Hi!
>> @@ -0,0 +1,119 @@
>> +/*
>> +* Copyright (c) 2016 Fujitsu Ltd.
>> +* Author: Jinbao Huang<huangjb.jy@cn.fujitsu.com>
>> +*
>> +* This program is free software; you can redistribute it and/or modify it
>> +* under the terms of version 2 of the GNU General Public License as
>> +* published by the Free Software Foundation.
>> +*
>> +* This program is distributed in the hope that it would be useful, but
>> +* WITHOUT ANY WARRANTY; without even the implied warranty of
>> +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>> +*
>> +* You should have received a copy of the GNU General Public License
>> +* alone with this program.
>> +*/
>> +
>> +/*
>> +* Test Name: lgetxattr01
>> +*
>> +* Description:
>> +* The testcase checks the basic functionality of the lgetxattr(2).
>> +* In the case of a symbolic link, we only get the value of the
>> +* extended attribute related to the link itself by name.
>> +*/
>> +
>> +#include "config.h"
>> +#include<errno.h>
>> +#include<sys/types.h>
>> +#include<string.h>
>> +
>> +#ifdef HAVE_ATTR_XATTR_H
>> +#include<attr/xattr.h>
>> +#endif
>> +
>> +#include "tst_test.h"
>> +
>> +#ifdef HAVE_ATTR_XATTR_H
>> +#define SECURITY_KEY1   "security.ltptest1"
>> +#define SECURITY_KEY2   "security.ltptest2"
>> +#define VALUE1   "test1"
>> +#define VALUE2   "test2"
>> +#define VALUE_SIZE      5
>> +
>> +static void set_xattr(char *path, char *key, void *value)
>> +{
>> +	int res;
>> +
>> +	res = lsetxattr(path, key, value, VALUE_SIZE, XATTR_CREATE);
>                                             ^
>                                            Should be either defined to
> 					  sizeof() or we can do strlen()
> 					  on the value as well.
>
>
> I would either add size parameter to this function so that it could be
> called in setup as:
>
> set_xattr("testfile", SECURITY_KEY1, VALUE1, sizeof(VALUE1));
>
> Which will include the terminating null character at the end, but I
> doubt that we care about if it's there or not.
>
> Or we can alternatively get the size with strlen(value) since it's a
> string.
>
>> +	if (res == -1) {
>> +		if (errno == ENOTSUP) {
>> +			tst_brk(TCONF,
>> +				"no xattr support in fs or mounted "
>> +				"without user_xattr option");
>> +		}
>> +
>> +		if (errno == EEXIST)
>> +			tst_brk(TBROK, "exist attribute %s", key);
>> +		else
>> +			tst_brk(TBROK | TERRNO, "lsetxattr() failed");
> Why do we have special branch for the EEXIST case here?
>
> We can just instead do:
>
> 	tst_brk(TBROK | TERRNO, "lsetxattr(%s)", key);
>
>> +	}
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	SAFE_TOUCH("testfile", 0644, NULL);
>> +	SAFE_SYMLINK("testfile", "symlink");
>> +
>> +	set_xattr("testfile", SECURITY_KEY1, VALUE1);
>> +	set_xattr("symlink", SECURITY_KEY2, VALUE2);
>> +}
>> +
>> +static void verify_lgetxattr(void)
>> +{
>> +	int size = 64;
>> +	char buf[size];
>> +
>> +	TEST(lgetxattr("symlink", SECURITY_KEY2, buf, size));
>> +	if (TEST_RETURN == -1) {
>> +		tst_res(TFAIL | TTERRNO, "lgetxattr() failed");
>> +		return;
>> +	}
>> +
>> +	if (TEST_RETURN != VALUE_SIZE) {
>> +		tst_res(TFAIL, "lgetxattr() got unexpected value size");
> We may as well try to print the buffer here somehow, possibly as
> hexadecimal data, since when we fail we will get pretty much random data
> . Unfortunately the tst_resm_hexd() is not implemented in the new
> library yet.
>
>> +		return;
>> +	}
> Well we can possibly do goto next; here instead of the return so that we
> do not skipp the next test if the first one fails, but I'm fine with
> code as it is as well.
>
>> +	if (!strncmp(buf, VALUE2, TEST_RETURN))
>> +		tst_res(TPASS, "lgetxattr() get expect value");
>> +	else
>> +		tst_res(TFAIL, "lgetxattr() got unexpected value");
>> +
>> +	TEST(lgetxattr("symlink", SECURITY_KEY1, buf, size));
>> +
>> +	if (TEST_RETURN != -1) {
>> +		tst_res(TFAIL, "lgetxattr() succeeded unexpectedly");
>> +		return;
>> +	}
>> +
>> +	if (TEST_ERRNO == ENODATA) {
>> +		tst_res(TPASS | TTERRNO, "lgetxattr() failed as expected");
>> +	} else {
>> +		tst_res(TFAIL | TTERRNO,
>> +			"lgetxattr() failed unexpectedly,"
>> +			"expected %s", tst_strerrno(ENODATA));
>> +	}
>> +}
>> +
>> +static struct tst_test test = {
>> +	.tid = "lgetxattr01",
>> +	.needs_tmpdir = 1,
>> +	.needs_root = 1,
>> +	.test_all = verify_lgetxattr,
>> +	.setup = setup,
>> +};
>> +
>> +#else
>> +	TST_TEST_TCONF("<attr/xattr.h>  does not exist.");
>> +#endif /* HAVE_ATTR_XATTR_H */
> Otherwise this looks good.
>

[LTP] [PATCH v2 1/2] lgetxattr/lgetxattr01.c: add new testcase

[Xiao Yang]

The testcase checks the basic functionality of the lgetxattr(2).
In the case of a symbolic link, we only get the value of the
extended attribute related to the link itself by name.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 runtest/ltplite                                   |   2 +
 runtest/syscalls                                  |   2 +
 testcases/kernel/syscalls/.gitignore              |   1 +
 testcases/kernel/syscalls/lgetxattr/Makefile      |  23 +++++
 testcases/kernel/syscalls/lgetxattr/lgetxattr01.c | 117 ++++++++++++++++++++++
 5 files changed, 145 insertions(+)
 create mode 100644 testcases/kernel/syscalls/lgetxattr/Makefile
 create mode 100644 testcases/kernel/syscalls/lgetxattr/lgetxattr01.c

diff --git a/runtest/ltplite b/runtest/ltplite
index 4f6186b..27ee584 100644
--- a/runtest/ltplite
+++ b/runtest/ltplite
@@ -385,6 +385,8 @@ lchown01 lchown01
 lchown02 lchown02
 lchown03 lchown03
 
+lgetxattr01 lgetxattr01
+
 link01 symlink01 -T link01
 link02 link02
 link03 link03
diff --git a/runtest/syscalls b/runtest/syscalls
index ed63358..44f6810 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -505,6 +505,8 @@ lchown03  lchown03
 lchown02_16 lchown02_16
 lchown03_16 lchown03_16
 
+lgetxattr01 lgetxattr01
+
 link01 symlink01 -T link01
 link02 link02
 link03 link03
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 0512f8a..9e4dcf0 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -470,6 +470,7 @@
 /lchown/lchown02_16
 /lchown/lchown03
 /lchown/lchown03_16
+/lgetxattr/lgetxattr01
 /link/link02
 /link/link03
 /link/link04
diff --git a/testcases/kernel/syscalls/lgetxattr/Makefile b/testcases/kernel/syscalls/lgetxattr/Makefile
new file mode 100644
index 0000000..b788fed
--- /dev/null
+++ b/testcases/kernel/syscalls/lgetxattr/Makefile
@@ -0,0 +1,23 @@
+#
+# Copyright (c) 2016 Fujitsu Ltd.
+# Author: Jinbao Huang <huangjb.jy@cn.fujitsu.com>
+#
+# This program is free software;  you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation;  either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY;  without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+# the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.
+#
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/lgetxattr/lgetxattr01.c b/testcases/kernel/syscalls/lgetxattr/lgetxattr01.c
new file mode 100644
index 0000000..5b0c137
--- /dev/null
+++ b/testcases/kernel/syscalls/lgetxattr/lgetxattr01.c
@@ -0,0 +1,117 @@
+/*
+* Copyright (c) 2016 Fujitsu Ltd.
+* Author: Jinbao Huang <huangjb.jy@cn.fujitsu.com>
+*
+* This program is free software; you can redistribute it and/or modify
+* it under the terms of version 2 of the GNU General Public License as
+* published by the Free Software Foundation.
+*
+* This program is distributed in the hope that it would be useful, but
+* WITHOUT ANY WARRANTY;  without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+*
+* You should have received a copy of the GNU General Public License
+* alone with this program.
+*/
+
+/*
+* Test Name: lgetxattr01
+*
+* Description:
+* The testcase checks the basic functionality of the lgetxattr(2).
+* In the case of a symbolic link, we only get the value of the
+* extended attribute related to the link itself by name.
+*
+*/
+
+#include "config.h"
+#include <errno.h>
+#include <sys/types.h>
+#include <string.h>
+
+#ifdef HAVE_ATTR_XATTR_H
+#include <attr/xattr.h>
+#endif
+
+#include "tst_test.h"
+
+#ifdef HAVE_ATTR_XATTR_H
+
+#define SECURITY_KEY1   "security.ltptest1"
+#define SECURITY_KEY2   "security.ltptest2"
+#define VALUE1   "test1"
+#define VALUE2   "test2"
+
+static void set_xattr(char *path, char *key, void *value, size_t size)
+{
+	int res;
+
+	res = lsetxattr(path, key, value, size, XATTR_CREATE);
+	if (res == -1) {
+		if (errno == ENOTSUP) {
+			tst_brk(TCONF,
+				"no xattr support in fs or mounted "
+				"without user_xattr option");
+		} else {
+			tst_brk(TBROK | TERRNO, "lsetxattr(%s) failed", key);
+		}
+	}
+}
+
+static void setup(void)
+{
+	SAFE_TOUCH("testfile", 0644, NULL);
+	SAFE_SYMLINK("testfile", "symlink");
+
+	set_xattr("testfile", SECURITY_KEY1, VALUE1, strlen(VALUE1));
+	set_xattr("symlink", SECURITY_KEY2, VALUE2, strlen(VALUE2));
+}
+
+static void verify_lgetxattr(void)
+{
+	int size = 64;
+	char buf[size];
+
+	TEST(lgetxattr("symlink", SECURITY_KEY2, buf, size));
+	if (TEST_RETURN == -1) {
+		tst_res(TFAIL | TTERRNO, "lgetxattr() failed");
+		goto next;
+	}
+
+	if (TEST_RETURN != strlen(VALUE2)) {
+		tst_res(TFAIL, "lgetxattr() got unexpected value size");
+		goto next;
+	}
+
+	if (!strncmp(buf, VALUE2, TEST_RETURN))
+		tst_res(TPASS, "lgetxattr() got expected value");
+	else
+		tst_res(TFAIL, "lgetxattr() got unexpected value");
+
+next:
+	TEST(lgetxattr("symlink", SECURITY_KEY1, buf, size));
+
+	if (TEST_RETURN != -1) {
+		tst_res(TFAIL, "lgetxattr() succeeded unexpectedly");
+		return;
+	}
+
+	if (TEST_ERRNO == ENODATA) {
+		tst_res(TPASS | TTERRNO, "lgetxattr() failed as expected");
+	} else {
+		tst_res(TFAIL | TTERRNO, "lgetxattr() failed unexpectedly,"
+			"expected %s", tst_strerrno(ENODATA));
+	}
+}
+
+static struct tst_test test = {
+	.tid = "lgetxattr01",
+	.needs_tmpdir = 1,
+	.needs_root = 1,
+	.test_all = verify_lgetxattr,
+	.setup = setup
+};
+
+#else
+	TST_TEST_TCONF("<attr/xattr.h> does not exist.");
+#endif /* HAVE_ATTR_XATTR_H */
-- 
1.8.3.1

[LTP] [PATCH v2 2/2] lgetxattr/lgetxattr02.c: add new testcase

[Xiao Yang]

1) lgetxattr(2) fails if the named attribute does not exist.
2) lgetxattr(2) fails if the size of the value buffer is too small
   to hold the result.
3) lgetxattr(2) fails when attemptes to read from a invalid address.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 runtest/ltplite                                   |   1 +
 runtest/syscalls                                  |   1 +
 testcases/kernel/syscalls/.gitignore              |   1 +
 testcases/kernel/syscalls/lgetxattr/lgetxattr02.c | 108 ++++++++++++++++++++++
 4 files changed, 111 insertions(+)
 create mode 100644 testcases/kernel/syscalls/lgetxattr/lgetxattr02.c

diff --git a/runtest/ltplite b/runtest/ltplite
index 27ee584..87dcc36 100644
--- a/runtest/ltplite
+++ b/runtest/ltplite
@@ -386,6 +386,7 @@ lchown02 lchown02
 lchown03 lchown03
 
 lgetxattr01 lgetxattr01
+lgetxattr02 lgetxattr02
 
 link01 symlink01 -T link01
 link02 link02
diff --git a/runtest/syscalls b/runtest/syscalls
index 44f6810..87fae55 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -506,6 +506,7 @@ lchown02_16 lchown02_16
 lchown03_16 lchown03_16
 
 lgetxattr01 lgetxattr01
+lgetxattr02 lgetxattr02
 
 link01 symlink01 -T link01
 link02 link02
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index 9e4dcf0..784cf1d 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -471,6 +471,7 @@
 /lchown/lchown03
 /lchown/lchown03_16
 /lgetxattr/lgetxattr01
+/lgetxattr/lgetxattr02
 /link/link02
 /link/link03
 /link/link04
diff --git a/testcases/kernel/syscalls/lgetxattr/lgetxattr02.c b/testcases/kernel/syscalls/lgetxattr/lgetxattr02.c
new file mode 100644
index 0000000..6d23469
--- /dev/null
+++ b/testcases/kernel/syscalls/lgetxattr/lgetxattr02.c
@@ -0,0 +1,108 @@
+/*
+* Copyright (c) 2016 Fujitsu Ltd.
+* Author: Jinbao Huang <huangjb.jy@cn.fujitsu.com>
+*
+* This program is free software; you can redistribute it and/or modify it
+* under the terms of version 2 of the GNU General Public License as
+* published by the Free Software Foundation.
+*
+* This program is distributed in the hope that it would be useful, but
+* WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+*
+* You should have received a copy of the GNU General Public License
+* alone with this program.
+*
+*/
+
+/*
+* Test Name: lgetxattr02
+*
+* Description:
+* 1) lgetxattr(2) fails if the named attribute does not exist.
+* 2) lgetxattr(2) fails if the size of the value buffer is too small
+*    to hold the result.
+* 3) lgetxattr(2) fails when attemptes to read from a invalid address.
+*
+* Expected Result:
+* 1) lgetxattr(2) should return -1 and set errno to ENOATTR.
+* 2) lgetxattr(2) should return -1 and set errno to ERANGE.
+* 3) lgetxattr(2) should return -1 and set errno to EFAULT.
+*/
+
+#include "config.h"
+#include <errno.h>
+#include <sys/types.h>
+#include <string.h>
+
+#ifdef HAVE_ATTR_XATTR_H
+#include <attr/xattr.h>
+#endif
+
+#include "tst_test.h"
+
+#ifdef HAVE_ATTR_XATTR_H
+
+#define SECURITY_KEY	"security.ltptest"
+#define VALUE	"this is a test value"
+
+static struct test_case {
+	const char *path;
+	size_t size;
+	int exp_err;
+} tcase[] = {
+	{"testfile", sizeof(VALUE), ENOATTR},
+	{"symlink", 1, ERANGE},
+	{(char *)-1, sizeof(VALUE), EFAULT}
+};
+
+static void verify_lgetxattr(unsigned int n)
+{
+	struct test_case *tc = tcase + n;
+	char buf[tc->size];
+
+	TEST(lgetxattr(tc->path, SECURITY_KEY, buf, sizeof(buf)));
+	if (TEST_RETURN != -1) {
+		tst_res(TFAIL, "lgetxattr() succeeded unexpectedly");
+		return;
+	}
+
+	if (TEST_ERRNO != tc->exp_err) {
+		tst_res(TFAIL | TTERRNO, "lgetxattr() failed unexpectedlly, "
+			"expected %s", tst_strerrno(tc->exp_err));
+	} else {
+		tst_res(TPASS | TTERRNO, "lgetxattr() failed as expected");
+	}
+}
+
+static void setup(void)
+{
+	int res;
+
+	SAFE_TOUCH("testfile", 0644, NULL);
+	SAFE_SYMLINK("testfile", "symlink");
+
+	res = lsetxattr("symlink", SECURITY_KEY, VALUE, strlen(VALUE), XATTR_CREATE);
+	if (res == -1) {
+		if (errno == ENOTSUP) {
+			tst_brk(TCONF, "no xattr support in fs or "
+				"mounted without user_xattr option");
+		} else {
+			tst_brk(TBROK | TERRNO, "lsetxattr(%s) failed",
+				SECURITY_KEY);
+		}
+	}
+}
+
+static struct tst_test test = {
+	.tid = "lgetxattr02",
+	.needs_tmpdir = 1,
+	.needs_root = 1,
+	.test = verify_lgetxattr,
+	.tcnt = ARRAY_SIZE(tcase),
+	.setup = setup
+};
+
+#else /* HAVE_ATTR_XATTR_H */
+	TST_TEST_TCONF("<attr/xattr.h> does not exist.");
+#endif
-- 
1.8.3.1

[LTP] [PATCH 1/2] lgetxattr/lgetxattr01.c: add new testcase

[Cyril Hrubis]

Hi!
> I'm afraid that huangjb.jy@cn.fujitsu.com no longer works as huang was 
> here only for his internship.
> Thanks for your review, and i will send these patches v2.

I've added Signed-of-by lines to the patch description for Jinbao Huang
as well (since he was the original author) and pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz