lkml.org 
[lkml]   [2009]   [Aug]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC][PATCH 4/5] tracing/filters: Provide basic regex support
    On Thu, Aug 06, 2009 at 11:14:57PM -0500, Tom Zanussi wrote:
    > Hi Frederic,
    >
    > On Thu, 2009-08-06 at 03:49 +0200, Frederic Weisbecker wrote:
    > > On Thu, Aug 06, 2009 at 09:14:27AM +0800, Li Zefan wrote:
    > > > Frederic Weisbecker wrote:
    > > > > On Mon, Aug 03, 2009 at 01:39:58PM +0800, Li Zefan wrote:
    > > > >> Frederic Weisbecker wrote:
    > > > >>> This patch provides basic support for regular expressions in filters.
    > > > >>> The common filter file doesn't support any regex but a new
    > > > >>> filter_regex file is created for each subsystem/event.
    > > > >>>
    > > > >>> It supports the following types of regexp:
    > > > >>>
    > > > >>> - *match_beginning
    > > > >>> - *match_middle*
    > > > >>> - match_end*
    > > > >>> - !don't match
    > > > >>>
    > > > >> I don't see why adding "filter_regex" is necessary, why not just make
    > > > >> "filter" support regex?
    > > > >
    > > > > I did that because I feared about people beeing unable to filter using
    > > > > * as a real character inside a filter string.
    > > > > If we are using only one file, we are forced to get the '*' interpreted.
    > > > > That would also break the ABI.
    > > > >
    > > >
    > > > I think we don't maintain stable ABI for debugfs and we've been changing
    > > > the "ABI" in debugfs/tracing/, but of course we shouldn't change it
    > > > without good reasons.
    > >
    > >
    > > Yeah. May be I'm too much careful there but i prefer not to take the risk.
    > >
    > >
    > > > One alternative is to use '"' to prevent '*' to be translated, but seems
    > > > using "filter_regex" is more convenient, so I agree with this patch.
    > >
    > >
    > > The problem is that most of the time, the '"' is about mandatory to delimit
    > > a string.
    > > Say you want to filter the lock name &REISERFS_SB(sb)->lock, you can't do that
    > > by just typing:
    > >
    > > echo name == &REISERFS_SB(sb)->lock > events/lockdep/filter
    > >
    > > because the '&' character is considered as an operator and that will trigger
    > > an error.
    > > Instead you must type:
    > >
    > > echo 'name == "&REISERFS_SB(sb)->lock"' > events/lockdep/filter
    > >
    > > The '' are there to delimit the string for echo, and the "" are interpreted
    > > by the filter api which take it as a whole string and not strings interleaved
    > > with operators. I even fear - and > may be considered as operators if we omit
    > > the "".
    > >
    > > I also thought about using an antislash for those who don't want the * to
    > > be interpreted. But although it seems intuitive, it's not as much as two
    > > distinct and visible files.
    > >
    > > Also, we can think about the fact the regexp support could be extended one
    > > day if someone needs to. And we could then encounter even more ambiguous
    > > characters such as $, ^, [, {, etc...
    > >
    >
    > This a nice new feature - I considered doing it (not complete regexp
    > support, just * in strings) for the original patch, but ran out of time
    > - glad you added it.
    >
    > I still think it makes sense to have some basic support for * in the
    > regular filter file, so I'd vote for getting rid of the filter_regex
    > file for now and just adding * support with the antislash escape to the
    > regular filter file. If you later wanted to add more full-fledged
    > regexp support and it didn't make sense to do it in the regular filter
    > file, then you could go crazy and add the filter_regex later...


    Ok, if you and Li both prefer the single file, I'm fine with that.
    It shouldn't really harm.
    I'll change that in the next version.

    Thanks.



    \
     
     \ /
      Last update: 2009-08-07 07:21    [W:0.027 / U:149.260 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site