On Wed, Mar 15, 2017 at 11:14 PM, Abhishek Tiwari <erabhishektiwar...@gmail.com> wrote: > On Tue, Mar 14, 2017 at 8:44 AM, Abhishek Tiwari > <erabhishektiwar...@gmail.com> wrote: >> On Tue, Mar 14, 2017 at 2:00 AM, Dmitry V. Levin <l...@altlinux.org> wrote: >>> On Tue, Mar 14, 2017 at 12:40:46AM +0530, Abhishek Tiwari wrote: >>>> > Per-file summary is not in GNU change log format, please refer to >>>> > https://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html . >>>> > Specifically, it lacks asterisks at the beginning of each new file >>>> > description and has excess spaces between file part and description, >>>> > otherwise looks fine. >>>> > >>>> > The patch wires up {,l}stat{,64}, fstatat/fstatat64 syscalls (used for >>>> > obtaining >>>> > file status), ustat syscall (which is a deprecated way to get some FS >>>> > statistics) and some statfs syscalls (which are more contemporary way >>>> > obtaining FS statistics information). On the other side, at least >>>> > fstat/fstat64, fstatfs/fstatfs64, old{,f,l}stat, osf_{,f}statfs, and >>>> > various >>>> > (mostly unsupported) syscalls with osf_, svr4_, sysv_, bsd43_, and posix_ >>>> > prefixes, present on alpha and mips, are omitted. I'm not sure whether >>>> > it was intended. >>>> > >>>> > There are minor tabulation irregularities introduced (at least) for >>>> > newfstatat and fstatat64 syscall entries, it is better to avoid this. >>>> >>>> I have fixed the commit entries. >>>> Now the patch includes all the stat-like syscalls that can be >>>> displayed by following command-> >>> >>> The idea is not to throw all syscalls that do some kind of stat into a >>> single class, but to create a class of syscalls that could be useful. >>> As Eugene already noted, there is little use of grouping syscalls that >>> stat files with those that stat filesystems. >>> >>> Actually, there is a software project that could benefit from more >>> fine-grained classes, e.g. oldstat+stat+stat64, oldlstat+lstat+lstat64, >>> and newfstatat+fstatat64. >> >> Ok, so oldstat+stat+stat64 is one class that is to be grouped under >> %stat, oldstat+lstat+lstat64 as another class and >> fstatat+newfstatat+fstatat64 and some other class. >> >> I read in mailing list it is intended to group syscalls that perform >> same action across different versions. >> >> But it is little confusing to decide what to group into one class. >> >> >>> There may be some use of a class that groups all filesystem stat syscalls >>> like ustat and *statfs*. This might appear to be easier for you to >>> implement, especially the part that tests it. >> >> I would be doing this part first. Grouping ustat and *statfs* under >> %statfs. Is this class name fine? >> >> >>>> grep -nr stat linux/*/syscallent* >>> >>> btw, why do you pass -nr to this grep command? >> >> This grep command is just to display the places where I have made the >> change. The command actually used is mentioned in commit. >>>> >> --- a/syscall.c >>>> >> +++ b/syscall.c >>>> >> @@ -77,6 +77,7 @@ >>>> >> #define TS TRACE_SIGNAL >>>> >> #define TM TRACE_MEMORY >>>> >> #define TSC TRACE_SCHED >>>> >> +#define TST TRACE_STAT >>>> > Tabulation is used for separating macro declaration and definition, but >>>> > neighbouring lines utilise space for this purpose. >>>> >>>> Please elaborate, i don't get what correction is intended here? >>> >>> The rule of thumb is simple: >>> when patching a piece of code, follow its coding style. >>> >>> If the code uses spaces, use spaces. If it uses tabs, use tabs. >>> strace has a long history with a lot of contributors so the style >>> used in different parts might differ slightly. >> >> Sorry, that space was not intensional. >> >>>> >> +++ b/tests/stat_like.test >>>> >> @@ -0,0 +1,58 @@ >>>> >> +#!/bin/sh >>>> >> + >>>> >> +# Check how stat-like syscalls are traced. >>>> >> +# >>>> >> +# Copyright (c) 2017 The strace developers. >>>> >> +# All rights reserved. >>>> >> +# >>>> >> +# Redistribution and use in source and binary forms, with or without >>>> >> +# modification, are permitted provided that the following conditions >>>> >> +# are met: >>>> >> +# 1. Redistributions of source code must retain the above copyright >>>> >> +# notice, this list of conditions and the following disclaimer. >>>> >> +# 2. Redistributions in binary form must reproduce the above copyright >>>> >> +# notice, this list of conditions and the following disclaimer in >>>> >> the >>>> >> +# documentation and/or other materials provided with the >>>> >> distribution. >>>> >> +# 3. The name of the author may not be used to endorse or promote >>>> >> products >>>> >> +# derived from this software without specific prior written >>>> >> permission. >>>> >> +# >>>> >> +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR >>>> >> +# IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED >>>> >> WARRANTIES >>>> >> +# OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE >>>> >> DISCLAIMED. >>>> >> +# IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, >>>> >> +# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, >>>> >> BUT >>>> >> +# NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF >>>> >> USE, >>>> >> +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY >>>> >> +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT >>>> >> +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE >>>> >> USE OF >>>> >> +# THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. >>>> >> + >>>> >> +STAT_TESTS='21 fstat >>>> >> +33 ustat >>>> >> +32 statx >>>> >> +17 statfs >>>> >> +21 oldfstat' >>>> >> + >>>> >> +NON_STAT_TESTS='11 fchdir >>>> >> +27 futex >>>> >> +10 fsync' >>>> >> + >>>> >> +. "${srcdir=.}/init.sh" >>>> >> + >>>> >> +echo "$STAT_TESTS" | while read w i >>>> >> +do >>>> >> + run_prog ./$i > /dev/null >>>> >> + run_strace -a$w -e%stat ./$i > "$EXP" >>>> >> + match_diff "$LOG" "$EXP" >>>> >> +done >>>> >> + >>>> >> +echo '+++ exited with 0 +++' > "$EXP" >>>> >> + >>>> >> +echo "$NON_STAT_TESTS" | while read w i >>>> >> +do >>>> >> + run_prog ./$i > /dev/null >>>> >> + run_strace -a$w -e%stat ./$i > /dev/null >>>> >> + match_diff "$LOG" "$EXP" >>>> >> +done >>>> >> + >>>> >> +rm "$EXP" >>>> > This test produces the following diagnostics: >>>> > >>>> > 1,2c1,2 >>>> > < fstat(0, 0x7f4dc9b2ffe0) = -1 EFAULT (Bad address) >>>> > < fstat(0, {st_dev=makedev(9, 1), st_ino=8761399, >>>> > st_mode=S_IFREG|0640, st_nlink=1, st_uid=1000, st_gid=1000, >>>> > st_blksize=4096, st_blocks=0, st_size=43147718418, >>>> > st_atime=1969-12-31T21:59:17+0100.000000135, >>>> > st_mtime=1969-12-31T21:59:19+0100.000000246, >>>> > st_ctime=2017-03-13T07:03:55+0100.962268762}) = 0 >>>> > --- >>>> > > stat("/etc/localtime", {st_mode=S_IFREG|0644, st_size=2246, ...}) >>>> > = 0 >>>> > > stat("/etc/localtime", {st_mode=S_IFREG|0644, st_size=2246, ...}) >>>> > = 0 >>>> > stat_like.test: failed test: ../strace -a21 -e%stat ./fstat output >>>> > mismatch >>>> > >>>> > It exits with status 0, however. >>>> >>>> Please help me with this part, how can I run this test on my machine. >>> >>> Do you ask about running this particular test? I think it's as simple as >>> $ make check TESTS=stat_like VERBOSE=1 >> >> Thank you, I just never did testing this way and I am new to this part. >>> >>> -- >>> ldv >>> >>> ------------------------------------------------------------------------------ >>> 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 >>> > > > Is it fine to group statfs+ statfs64+ fstatfs + fstatfs64 + ustat as > %statfs or it should be (statfs+statfs64 + ustat) and > (fstatfs+ftsatfs64) i.e. two different classes ?
Please reply ------------------------------------------------------------------------------ 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
