GSOC: Introduction

Thu Mar 16 16:37:27 UTC 2017

On Thu, Mar 16, 2017 at 9:53 AM, Abhishek Tiwari
<erabhishektiwarics at gmail.com> wrote:
> On Wed, Mar 15, 2017 at 11:14 PM, Abhishek Tiwari
> <erabhishektiwarics at gmail.com> wrote:
>> On Tue, Mar 14, 2017 at 8:44 AM, Abhishek Tiwari
>> <erabhishektiwarics at gmail.com> wrote:
>>> On Tue, Mar 14, 2017 at 2:00 AM, Dmitry V. Levin <ldv at 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 at 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


Implemented -e trace=%statfs option. It groups statfs+statfs64+ustat
Below is patch attached. Please review.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v4-0001-Implement-e-trace-statfs-option-for-tracing-statf.patch
Type: text/x-patch
Size: 43815 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20170316/84bafddf/attachment.bin>