On Wed, Mar 15, 2017 at 02:57:02PM +0300, Victor Krapivensky wrote: > * Makefile.am (strace_SOURCES): Add statx.c and stat.h.
There is a shorter form for this:
* statx.c: New file.
* statx.h: Likewise.
* Makefile.am (strace_SOURCES): Add them.
> * linux/i386/syscallent.h: Add statx entry.
> * linux/x32/syscallent.h: Likewise.
> * linux/x86_64/syscallent.h: Likewise.
The current practice is either add a number, e.g.
* linux/i386/syscallent.h [383]: Add statx entry.
or add a name, e.g.
* linux/i386/syscallent.h (statx): New entry.
[...]
> + printxvals(tcp->u_arg[2] & AT_STATX_SYNC_TYPE, "AT_STATX_???",
> + at_statx_sync_types, NULL);
I think printxval would be easier to understand than printxvals.
[...]
> +#include "tests.h"
> +#include <asm/unistd.h>
> +#include <linux/stat.h>
> +#include "xlat.h"
> +#include "xlat/statx_masks.h"
> +#include "xlat/statx_attrs.h"
> +#include "xlat/at_statx_sync_types.h"
> +
> +#ifdef __NR_statx
The current practice is to place this ifdef right after <asm/unistd.h>.
[...]
Let's add "static" to all these variables.
> +
> +# define TEST_SYSCALL_INVOKE(sample, pst) \
> + syscall(__NR_statx, AT_FDCWD, sample, TEST_SYSCALL_STATX_FLAGS, \
> + TEST_SYSCALL_STATX_MASK, pst)
> +# define PRINT_SYSCALL_HEADER(sample) \
> + do { \
> + int saved_errno = errno; \
> + printf("%s(AT_FDCWD, \"%s\", %s, %s, ", \
> + TEST_SYSCALL_STR, sample, TEST_SYSCALL_STATX_FLAGS_STR, \
> + TEST_SYSCALL_STATX_MASK_STR)
> +# define PRINT_SYSCALL_FOOTER(rc) \
> + errno = saved_errno; \
> + printf(") = %s\n", sprintrc(rc)); \
> + } while (0)
> +
> +# include "xstatx.c"
> +
> +#else
> +
> +SKIP_MAIN_UNDEFINED("__NR_statx")
> +
> +#endif
> +
This blank line at the end has caused this "git am" response:
Applying: Implement decoding of statx syscall
.git/rebase-apply/patch:385: new blank line at EOF.
+
fatal: 1 line adds whitespace errors.
Patch failed at 0001 Implement decoding of statx syscall
Please don't. :)
[...]
> @@ -246,19 +305,26 @@ main(void)
> return 77;
> }
> }
> - (void) unlink(sample);
If you move this unlink invocation, please add it on error paths.
> +# if IS_STATX
> +# define ST_SIZE_FIELD stx_size
> +# else
> +# define ST_SIZE_FIELD st_size
> +# endif
> if (!rc && zero_extend_signed_to_ull(SAMPLE_SIZE) !=
> - zero_extend_signed_to_ull(st[0].st_size)) {
> + zero_extend_signed_to_ull(st[0].ST_SIZE_FIELD)) {
> fprintf(stderr, "Size mismatch: "
> "requested size(%llu) != st_size(%llu)\n",
> zero_extend_signed_to_ull(SAMPLE_SIZE),
> - zero_extend_signed_to_ull(st[0].st_size));
> + zero_extend_signed_to_ull(st[0].ST_SIZE_FIELD));
> fprintf(stderr, "The most likely reason for this is incorrect"
> " definition of %s.\n"
> "Here is some diagnostics that might help:\n",
> STRUCT_STAT_STR);
There is no diagnostics in case of IS_STATX.
Someone seems to be too lazy to split the message. :)
> -#define LOG_STAT_OFFSETOF_SIZEOF(object, member) \
> +# if !IS_STATX
> +
> +# define LOG_STAT_OFFSETOF_SIZEOF(object, member) \
> fprintf(stderr, "offsetof(%s, %s) = %zu" \
> ", sizeof(%s) = %zu\n", \
> STRUCT_STAT_STR, #member, \
> @@ -273,10 +339,12 @@ main(void)
> LOG_STAT_OFFSETOF_SIZEOF(st[0], st_gid);
> LOG_STAT_OFFSETOF_SIZEOF(st[0], st_rdev);
> LOG_STAT_OFFSETOF_SIZEOF(st[0], st_size);
> -# if !OLD_STAT
> +# if !OLD_STAT
> LOG_STAT_OFFSETOF_SIZEOF(st[0], st_blksize);
> LOG_STAT_OFFSETOF_SIZEOF(st[0], st_blocks);
> -# endif /* !OLD_STAT */
> +# endif /* !OLD_STAT */
> +
> +# endif /* !IS_STATX */
>
> return 1;
Please add that unlink(sample) call here.
> }
> @@ -288,6 +356,74 @@ main(void)
> print_stat(st);
> PRINT_SYSCALL_FOOTER(rc);
>
> +# if IS_STATX
> +
> + // Check that our idea of struct_statx is no smaller than the kernel's.
> + // (If it is, statx is expected to fail with EFAULT.)
This is not the way we do multiline comments, please convert.
The last but not least, please add a line to NEWS file.
On the whole, this is very close to what I'd like to see as a statx parser.
I've removed it from the list of available microprojects.
--
ldv
pgpOcJwI7Nk2r.pgp
Description: PGP signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Strace-devel mailing list Strace-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/strace-devel
