[PATCH] tests: add chmod.test

[Dmitry V. Levin]

On Fri, Mar 04, 2016 at 06:25:59PM +0000, Anchit Jain wrote:
> diff --git a/tests/chmod.c b/tests/chmod.c
> new file mode 100644
> index 0000000..bdf53d7
> --- /dev/null
> +++ b/tests/chmod.c
> @@ -0,0 +1,51 @@
> +/*
> + * Copyright (c) 2015-2016 Dmitry V. Levin <ldv at altlinux.org>
> + * All rights reserved.

No, this cannot be correct, I didn't write this file. :)

> +#include "tests.h"
> +#include <assert.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <sys/syscall.h>
> +
> +int
> +main(void)
> +{
> +	int create_file_desc = open("/tmp/chmod_test_file", O_CREAT, 0400);

Please don't create any files in public directories.
In this case, current directory is perfectly OK.
When creating files, please specify an access mode, e.g. O_RDONLY.

> +	if(create_file_desc == -1)
> +		perror_msg_and_skip("chmod -1");

Error creating a file is a framework failure, that's a reason for a fail
rather than skip.

> +	assert(syscall(__NR_chmod, "/tmp/chmod_test_file", 0600) == 0);
> +	printf("chmod(\"/tmp/chmod_test_file\", 0600) = 0\n");

When a syscall shouldn't fail, use perror_msg_and_fail instead of assert.
In this case, __NR_chmod may fail because of ENOSYS, so you have to handle
a potential error.  More than that, __NR_chmod is not even defined on
modern architectures, so you have to guard the test against it.

> +	remove("/tmp/chmod_test_file");

The object being removed is a regular file, so please use unlink instead.

> +	assert(syscall(__NR_chmod, "/dev/null", 0600) == -1);

If by any chance this syscall succeeds (e.g. if run by a privileged user),
it will render the whole system unusable by unprivileged users.
Never assume you cannot change a system object.

Assuming you want to test how chmod error is printed, go for ENOENT,
it's harmless and easy to reproduce.

> +	printf("chmod(\"/dev/null\", 0600)    = -1 EPERM (%m)\n");

The tradition is to use one space before "=" and specify -a option for
this effect in the test driver.  The idea behind this tradition is not
to assume that DEFAULT_ACOLUMN has any particular value.

> diff --git a/tests/chmod.test b/tests/chmod.test
> new file mode 100755
> index 0000000..15e1f5e
> --- /dev/null
> +++ b/tests/chmod.test
> @@ -0,0 +1,13 @@
> +#!/bin/sh
> +
> +# Check epoll_create1 syscall decoding.

epoll_create1 syscall?  Really??


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20160304/9225ba0c/attachment.bin>