On Fri, Mar 04, 2016 at 07:26:35PM +0000, Anchit Jain wrote:
> diff --git a/tests/chmod.c b/tests/chmod.c
> new file mode 100644
> index 0000000..b6bb2fd
> --- /dev/null
> +++ b/tests/chmod.c
> @@ -0,0 +1,62 @@
> +/*
> + * Copyright (c) 2015-2016 Anchit Jain <anchitjain1...@gmail.com>
> + * All rights reserved.

Have you really started working on this test last year?

> +     int create_file_desc = open("chmod_test_file", O_CREAT|O_RDONLY, 0400);

Do you plan to use other descriptors besides this one?
If not, "create_file_desc" is too verbose.

> +     if(create_file_desc == -1)

Please add a space after "if" in all "if" statements.

> +             perror_msg_and_fail("chmod -1");

What does this "chmod -1" mean?

> +     if(syscall(__NR_chmod, "chmod_test_file", 0600) != 0)
> +             perror_msg_and_fail("chmod -1");


Let me repeat: chmod syscall can legitimately fail with ENOSYS.

> +     unlink("chmod_test_file");

Let's practice defensive programming and check unlink return code as well.

> +     if(syscall(__NR_chmod, "chmod_test_file", 0600) != -1)
> +             perror_msg_and_fail("chmod -1");

What does "chmod -1" mean this time?

> +     printf("chmod(\"chmod_test_file\", 0600) = -1 ENOENT (%m)\n");

Parametrising the name of test file might make the whole test more
readable.  Assuming you have
        static const char fname[] = "chmod_test_file";
defined earlier in the test, all statements that use this test file name
will be easier to read.  Compare e.g.
        syscall(__NR_chmod, "chmod_test_file", 0600)
with
        syscall(__NR_chmod, fname, 0600)
and
        printf("chmod(\"chmod_test_file\", 0600) = -1 ENOENT (%m)\n");
with
        printf("chmod(\"%s\", 0600) = -1 ENOENT (%m)\n", fname);

> +#endif
> \ No newline at end of file

Why is that?


-- 
ldv

Attachment: pgpIQ74VdOmYF.pgp
Description: PGP signature

------------------------------------------------------------------------------
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel

Reply via email to