Re: [PATCH] Modify mtd-utils intgck utility to test extended attribute set/get for UBIFS

[Artem Bityutskiy <dedekind1@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2926 bytes --]

On Wed, 2012-04-11 at 11:24 -0700, subodh.nijsure@gmail.com wrote:
> From: Subodh Nijsure <snijsure@grid-net.com>
> 
> [ Assuming that linux-mtd is right place to send patches for mtd-utils]
> 
> Need to compile the test target with option UBIFS_XATTR_TEST=1
> run make as "UBIFS_XATTR_TEST=1 make tests"
> 
> Signed-off-by: Subodh Nijsure <snijsure@grid-net.com>
> ---
>  tests/fs-tests/integrity/Makefile  |   11 +++++++++++
>  tests/fs-tests/integrity/integck.c |   31 +++++++++++++++++++++++++++++++
>  tests/ubi-tests/Makefile           |    3 ++-
>  3 files changed, 44 insertions(+), 1 deletions(-)
> 
> diff --git a/tests/fs-tests/integrity/Makefile b/tests/fs-tests/integrity/Makefile
> index 4d6fc7d..2b1280f 100644
> --- a/tests/fs-tests/integrity/Makefile
> +++ b/tests/fs-tests/integrity/Makefile
> @@ -3,6 +3,12 @@ ifeq ($(origin CC),default)
>  CC = gcc
>  endif
>  
> +#To compile  integck with XATTR test support
> +#invoke as UBIFS_XATTR_TEST=1 make tests from toplevel directory
> +ifeq ($(UBIFS_XATTR_TEST), 1)
> +  CPPFLAGS += -DUBIFS_XATTR_TEST
> +endif

Would you please make this unconditional, i.e., remove the
UBIFS_XATTR_TEST thing completely.

> @@ -38,6 +38,7 @@
>  #include <sys/statvfs.h>
>  #include <linux/fs.h>
>  
> +
>  #define PROGRAM_VERSION "1.1"

Do not add extra newlines please.

> +/* Assign given path extended attribute security.selinx value
> + * root:object_t:bin_t then read it back and verify it correct
> + */
> +#ifdef UBIFS_XATTR_TEST

Please, kill this ifdef as well.

> +void test_xattr(char *path)
> +{
> +	int ret;
> +	char buf[255];
> +	char value[255];
> +	int attrLen;
> +	strcpy(value,"root:object_r:bin_t");
> +	attrLen = strlen(value) + 1;
> +	ret = setxattr (path, "security.selinux", value, attrLen, 0x0);
> +	v("assign extended attribute to %s", path);
> +	CHECK(ret == 0);
> +	if ( ret == 0 ) {

CHECK(ret == 0) will kill the test if ret != 0, so the above "if"
statement is redundant - please kill it.
> +		v("retrieve extended attribute for  %s", path);
> +		ret = getxattr(path,"security.selinux",buf,attrLen);
> +		CHECK(strncmp(buf,"root:object_r:bin_t", attrLen) == 0 );
> +	}
> +}
> +#endif

The patch is OK except of few nitpicks, but it would be much better if
you also remembered the extended addributes you created.

How the integck test works it creates random objects on the FS and
remembers them in internal in-memory data structures. Then it unmounts
the FS, mounts back, and checks that the objects are there. Would be
much better if you remembered and checked xattrs too.

> 	add_dir_entry(parent, 'd', name, NULL);
> @@ -627,6 +653,8 @@ static int dir_remove(struct dir_info *dir)
>  	return 0;
>  }
>  
> +
> +
>  static int file_new(struct dir_info *parent, const char *name)

Junk extra newlines..

Thanks!

-- 
Best Regards,
Artem Bityutskiy


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]