Re: [PATCH 13/14] tools lib traceevent: Refactor test_filter() to get rid of die()

[Namhyung Kim]

On Mon, 9 Dec 2013 11:19:36 -0500, Steven Rostedt wrote:
> On Mon,  9 Dec 2013 14:34:10 +0900
> Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
>> @@ -1788,39 +1790,44 @@ get_exp_value(struct event_format *event, struct filter_arg *arg, struct pevent_
>>  
>>  	case FILTER_EXP_NOT:
>>  	default:
>> -		die("error in exp");
>> +		if (*error_str == NULL)
>> +			*error_str = "invalid expression type";
>
> Hmm, how do we tell the caller that there was an error? Do they just
> check to see if error_str was changed?
>
>>  	}
>>  	return 0;
>>  }
[SNIP]
>> @@ -2004,6 +2021,7 @@ int pevent_event_filtered(struct event_filter *filter,
>>   *  0 - filter found for event and @record does not match
>>   * -1 - no filter found for @record's event
>>   * -2 - if no filters exist
>> + * -3 - if error occurred during test
>>   */
>>  int pevent_filter_match(struct event_filter *filter,
>>  			struct pevent_record *record)
>> @@ -2011,6 +2029,8 @@ int pevent_filter_match(struct event_filter *filter,
>>  	struct pevent *pevent = filter->pevent;
>>  	struct filter_type *filter_type;
>>  	int event_id;
>> +	char *error_str = NULL;
>> +	int ret;
>>  
>>  	if (!filter->filters)
>>  		return FILTER_NONE;
>> @@ -2022,8 +2042,14 @@ int pevent_filter_match(struct event_filter *filter,
>>  	if (!filter_type)
>>  		return FILTER_NOEXIST;
>>  
>> -	return test_filter(filter_type->event, filter_type->filter, record) ?
>> -		FILTER_MATCH : FILTER_MISS;
>> +	ret = test_filter(filter_type->event, filter_type->filter, record,
>> +			  &error_str);
>> +	if (error_str) {
>> +		/* TODO: maybe we can print it or pass back to user */
>
> Ah, I guess this answers my question :-)

Right.  I was also considering what's the best way to handle error..

>
> Maybe we can save the error_str in the pevent. Then we can extract it
> later. The return of FILTER_ERROR will let the user see what happened.

Yes, but then we need the extraction and free-ing APIs too.

Or else, we can do similar to pevent_errno/strerror - returns a specific
error code and print it with user-supplied buffer.  If so we need to
think about whether consolidating it with existing pevent API or making
it a separate filter-specific API IMHO.

Thanks,
Namhyung

>
>> +		return FILTER_ERROR;
>> +	}
>> +
>> +	return ret ? FILTER_MATCH : FILTER_MISS;
>>  }
>>  
>>  static char *op_to_str(struct event_filter *filter, struct filter_arg *arg)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/