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 <[email protected]> > + * 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
pgpIQ74VdOmYF.pgp
Description: PGP signature
------------------------------------------------------------------------------
_______________________________________________ Strace-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/strace-devel
