[PATCH v6] Implement -e trace=%statfs option for tracing statfs like syscalls

[Dmitry V. Levin]

On Sat, Mar 18, 2017 at 08:29:46PM +0530, Abhishek Tiwari wrote:
> This commit adds %statsfs option to trace statfs, statfs64, statvfs, statvfs64 syscalls. The following commands update linux/*/syscallent* files:

This line is too long.  The limit is 72 symbols, please wrap.

> 	git grep -Fl 'statfs' linux/*/syscallent* | xargs sed -i 's/TD\(,[[:space:]]*SEN(statfs\)/TD|TSF\1/'

Do you really need this?

$ git grep 'TD,[[:space:]]*SEN(statfs' linux |wc -l
0

> 	git grep -Fl 'statfs' linux/*/syscallent* | xargs sed -i 's/TF\(,[[:space:]]*SEN(statfs\)/TF|TSF\1/'

This line is too long.  As this command has been copied from commit
v4.16-23-g811638e, please copy the indentation, too.

> 	git grep -Fl 'statfs' linux/*/syscallent* | xargs sed -i 's/0\(,[[:space:]]*SEN(osf_statfs\)/TSF\1/'
> 	git grep -Fl 'statfs' linux/*/syscallent* | xargs sed -i 's/0\(,[[:space:]]*SEN(.*_statfs\)/TSF\1/'
> 	git grep -Fl 'statvfs' linux/*/syscallent* | xargs sed -i 's/0\(,[[:space:]]*SEN(.*_statvfs\)/TSF\1/'

Why do you need 3 commands where a single command would suffice?

Why these entries don't have TF flag set in the first place?
It's a bug that should be fixed with a separate commit.

[...]
> * tests/ksysent.c: Define TST to 0.
> * tests/nsyscalls.c: Likewise.

Why TST?

[...]
> --- a/strace.1
> +++ b/strace.1
> @@ -429,6 +429,9 @@ Trace all memory mapping related system calls.
>  .BR "\-e\ trace" = %sched
>  Trace all scheduler-related (sched_*) system calls.
>  .TP
> +.BR "\-e\ trace" = %statfs
> +Trace statfs-related (statfs{,64}) system calls.

As these two syscalls are not all known statfs-related syscalls and we are
going to have more statfs-related classes soon, please change the wording
to avoid this confusion with %fstatfs and %%statfs.

[...]
> --- a/sysent.h
> +++ b/sysent.h
> @@ -22,5 +22,6 @@ typedef struct sysent {
>  #define TRACE_INDIRECT_SUBCALL	02000	/* Syscall is an indirect socket/ipc subcall. */
>  #define COMPAT_SYSCALL_TYPES	04000	/* A compat syscall that uses compat types. */
>  #define TRACE_SCHED		010000  /* Trace scheduler-related syscalls. */
> +#define TRACE_STATFS	040000  /* Trace narrower statfs-related syscalls. */

Please adjust indentation.

[...]
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -734,6 +734,7 @@ DECODER_TESTS = \
>  	rt_sigtimedwait.test \
>  	rt_tgsigqueueinfo.test \
>  	sched.test \
> +	trace_statfs.test \
>  	sched_get_priority_mxx.test \
>  	sched_rr_get_interval.test \
>  	sched_xetaffinity.test \

Please keep the list sorted.

[...]
> +done << EOF
> +17 statfs

Please add a test for statfs64 here as well.

[...]
> +done << EOF
> +11 fchdir
> +28 futex
> +10 fsync

Please add more tests here, too.


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