Re: [PATCH 01/14] tools lib traceevent: Get rid of malloc_or_die() in show_error()

From: Namhyung Kim
Date: Tue Dec 10 2013 - 21:29:31 EST


On Tue, 10 Dec 2013 20:55:26 -0500, Steven Rostedt wrote:
> On Wed, 11 Dec 2013 09:40:35 +0900
> Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
>> And should we extend the error code to include the return value of
>> pevent_filter_match() too? If not, it seems we need to pass another
>> argument to receive the actual error code in case of FILTER_ERROR.
>
> I'm a bit confused on this. Perhaps it's something you added in your
> patches. If what returns FILTER_ERROR?

Well, I mean there are some cases which return FILTER_ERROR. With my
patch, test_filter() can failed with the error_str set like following:

- invalid expression type
- must have number field
- invalid numeric argument type
- invalid numeric comparison type
- invalid string comparison type
- invalid operator type
- invalid argument type

To distinguish them, we either need to extend return value or another
argument. But the current return value of pevent_filter_match() was
defined as FILTER_{MATCH,MISS,NOEXIST,NONE,ERROR}.

And also I want all user APIs share same return value/type as
pevent_errno so that user can pass it our strerror function to see the
error message.

So to use return value, we need to extend the error code to include all
possible error cases above as well as normal cases (MATCH, MISS, ...).

>
>>
>> I'm saying these here since they might require interface/signature
>> change so will affect existing users like trace-cmd.
>
> I'm OK if they change now. I'll have trace-cmd and other users adapt.
> As each currently has their own copy. I've been updating trace-cmd with
> what's in tools for a while now, and plan to continue doing that until
> we have something that seems good for a public library.

Okay, I'll cook the patch soon!

Thanks,
Namhyung
--
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/