lkml.org 
[lkml]   [2009]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 4/7] tracing/filters: allow user to specify a filter val to be string

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> Hi Li,
>
> On Sat, Apr 11, 2009 at 03:53:05PM +0800, Li Zefan wrote:
> > Suppose we would like to trace all tasks named '123', but this
> > will fail:
> > # echo 'parent_comm == 123' > events/sched/sched_process_fork/filter
> > bash: echo: write error: Invalid argument
> >
> > With this patch, we allow it by:
> > # echo 'parent_comm == \123' > events/sched/sched_process_fork/filter
> > # cat events/sched/sched_process_fork/filter
> > parent_comm == 123
>
> Well, IMHO, it would be rather better to just echo
> 'parent_comm == 123' and let it answer depending of which
> filter_pred_*() callback we have for the concerned field.
>
>
> The culprit is this part in filter_parse():
>
> pred->val = simple_strtoull(val_str, &tmp, 10);
> if (tmp == val_str) {
> pred->str_val = kstrdup(val_str, GFP_KERNEL);
> if (!pred->str_val)
> return -ENOMEM;
> }
>
> The idea would be to not anymore base the check on simple_strtoull to
> guess whether this is a number or a string, making it act subsequently
> to this assumption, which is not the good assumption we must base our
> parsing yet.
>
> Instead, we could let filter_parse only do the job of extracting the tokens
> and then fill the whole pred struct without yet bothering about the type
> of the value.
>
> Thereafter we may defer the real value type checking on filter_add_pred()
> depending on the type of the concerned field:
>
> if (is_string_field()) {
> add it as a string value;
> } else {
> do the check with simple_strtoull
> looks good? Then go to the number size switch....
> ...
> }
>
> You see?
>
> I think it would be a saner basis of parsing.

Agreed. The user input itself is unambigously parse-able -
and we should not work around parsing limitations by
complicating the user input.

The failure situation that Li found should obviously be
fixed.

Ingo


\
 
 \ /
  Last update: 2009-04-12 12:07    [W:0.051 / U:1.740 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site