Messages in this thread | | | Subject | Re: [RFC][PATCH 4/5] tracing/filters: Provide basic regex support | From | Tom Zanussi <> | Date | Thu, 06 Aug 2009 23:14:57 -0500 |
| |
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...
Thanks,
Tom
| |