[PATCH] tcp: Export to userspace the TCP state names for the trace events
From: "Steven Rostedt (VMware)"
The TCP trace events (specifically tcp_set_state), maps emums to symbol
names via __print_symbolic(). But this only works for reading trace events
from the tracefs trace files. If perf or trace-cmd were to record these
events, the event format file does not convert the enum names into numbers,
and you get something like:
__print_symbolic(REC->oldstate,
{ TCP_ESTABLISHED, "TCP_ESTABLISHED" },
{ TCP_SYN_SENT, "TCP_SYN_SENT" },
{ TCP_SYN_RECV, "TCP_SYN_RECV" },
{ TCP_FIN_WAIT1, "TCP_FIN_WAIT1" },
{ TCP_FIN_WAIT2, "TCP_FIN_WAIT2" },
{ TCP_TIME_WAIT, "TCP_TIME_WAIT" },
{ TCP_CLOSE, "TCP_CLOSE" },
{ TCP_CLOSE_WAIT, "TCP_CLOSE_WAIT" },
{ TCP_LAST_ACK, "TCP_LAST_ACK" },
{ TCP_LISTEN, "TCP_LISTEN" },
{ TCP_CLOSING, "TCP_CLOSING" },
{ TCP_NEW_SYN_RECV, "TCP_NEW_SYN_RECV" })
Where trace-cmd and perf do not know the values of those enums.
Use the TRACE_DEFINE_ENUM() macros that will have the trace events convert
the enum strings into their values at system boot. This will allow perf and
trace-cmd to see actual numbers and not enums:
__print_symbolic(REC->oldstate,
{ 1, "TCP_ESTABLISHED" },
{ 2, "TCP_SYN_SENT" },
{ 3, "TCP_SYN_RECV" },
{ 4, "TCP_FIN_WAIT1" },
{ 5, "TCP_FIN_WAIT2" },
{ 6, "TCP_TIME_WAIT" },
{ 7, "TCP_CLOSE" },
{ 8, "TCP_CLOSE_WAIT" },
{ 9, "TCP_LAST_ACK" },
{ 10, "TCP_LISTEN" },
{ 11, "TCP_CLOSING" },
{ 12, "TCP_NEW_SYN_RECV" })
Signed-off-by: Steven Rostedt (VMware)
---
include/trace/events/tcp.h | 41 -
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
index 07a6cbf1..62e5bad7901f 100644
--- a/include/trace/events/tcp.h
+++ b/include/trace/events/tcp.h
@@ -9,21 +9,36 @@
#include
#include
+#define tcp_state_names\
+ EM(TCP_ESTABLISHED) \
+ EM(TCP_SYN_SENT)\
+ EM(TCP_SYN_RECV)\
+ EM(TCP_FIN_WAIT1) \
+ EM(TCP_FIN_WAIT2) \
+ EM(TCP_TIME_WAIT) \
+ EM(TCP_CLOSE) \
+ EM(TCP_CLOSE_WAIT) \
+ EM(TCP_LAST_ACK)\
+ EM(TCP_LISTEN) \
+ EM(TCP_CLOSING) \
+ EMe(TCP_NEW_SYN_RECV)
+
+/* enums need to be exported to user space */
+#undef EM
+#undef EMe
+#define EM(a) TRACE_DEFINE_ENUM(a);
+#define EMe(a)TRACE_DEFINE_ENUM(a);
+
+tcp_state_names
+
+#undef EM
+#undef EMe
+#define EM(a) tcp_state_name(a),
+#define EMe(a)tcp_state_name(a)
+
#define tcp_state_name(state) { state, #state }
#define show_tcp_state_name(val) \
- __print_symbolic(val, \
- tcp_state_name(TCP_ESTABLISHED),\
- tcp_state_name(TCP_SYN_SENT), \
- tcp_state_name(TCP_SYN_RECV), \
- tcp_state_name(TCP_FIN_WAIT1), \
- tcp_state_name(TCP_FIN_WAIT2), \
- tcp_state_name(TCP_TIME_WAIT), \
- tcp_state_name(TCP_CLOSE), \
- tcp_state_name(TCP_CLOSE_WAIT), \
- tcp_state_name(TCP_LAST_ACK), \
- tcp_state_name(TCP_LISTEN), \
- tcp_state_name(TCP_CLOSING),\
- tcp_state_name(TCP_NEW_SYN_RECV))
+ __print_symbolic(val, tcp_state_names)
/*
* tcp event with arguments sk and skb
--
2.13.6
Re: [PATCH] tcp: Export to userspace the TCP state names for the trace events
> On Nov 9, 2017, at 4:57 PM, Steven Rostedt wrote:
>
>
> From: "Steven Rostedt (VMware)"
>
> The TCP trace events (specifically tcp_set_state), maps emums to symbol
> names via __print_symbolic(). But this only works for reading trace events
> from the tracefs trace files. If perf or trace-cmd were to record these
> events, the event format file does not convert the enum names into numbers,
> and you get something like:
>
> __print_symbolic(REC->oldstate,
>{ TCP_ESTABLISHED, "TCP_ESTABLISHED" },
>{ TCP_SYN_SENT, "TCP_SYN_SENT" },
>{ TCP_SYN_RECV, "TCP_SYN_RECV" },
>{ TCP_FIN_WAIT1, "TCP_FIN_WAIT1" },
>{ TCP_FIN_WAIT2, "TCP_FIN_WAIT2" },
>{ TCP_TIME_WAIT, "TCP_TIME_WAIT" },
>{ TCP_CLOSE, "TCP_CLOSE" },
>{ TCP_CLOSE_WAIT, "TCP_CLOSE_WAIT" },
>{ TCP_LAST_ACK, "TCP_LAST_ACK" },
>{ TCP_LISTEN, "TCP_LISTEN" },
>{ TCP_CLOSING, "TCP_CLOSING" },
>{ TCP_NEW_SYN_RECV, "TCP_NEW_SYN_RECV" })
>
> Where trace-cmd and perf do not know the values of those enums.
>
> Use the TRACE_DEFINE_ENUM() macros that will have the trace events convert
> the enum strings into their values at system boot. This will allow perf and
> trace-cmd to see actual numbers and not enums:
>
> __print_symbolic(REC->oldstate,
>{ 1, "TCP_ESTABLISHED" },
>{ 2, "TCP_SYN_SENT" },
>{ 3, "TCP_SYN_RECV" },
>{ 4, "TCP_FIN_WAIT1" },
>{ 5, "TCP_FIN_WAIT2" },
>{ 6, "TCP_TIME_WAIT" },
>{ 7, "TCP_CLOSE" },
>{ 8, "TCP_CLOSE_WAIT" },
>{ 9, "TCP_LAST_ACK" },
>{ 10, "TCP_LISTEN" },
>{ 11, "TCP_CLOSING" },
>{ 12, "TCP_NEW_SYN_RECV" })
>
> Signed-off-by: Steven Rostedt (VMware)
> ---
> include/trace/events/tcp.h | 41 -
> 1 file changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
> index 07a6cbf1..62e5bad7901f 100644
> --- a/include/trace/events/tcp.h
> +++ b/include/trace/events/tcp.h
> @@ -9,21 +9,36 @@
> #include
> #include
>
> +#define tcp_state_names \
> + EM(TCP_ESTABLISHED) \
> + EM(TCP_SYN_SENT)\
> + EM(TCP_SYN_RECV)\
> + EM(TCP_FIN_WAIT1) \
> + EM(TCP_FIN_WAIT2) \
> + EM(TCP_TIME_WAIT) \
> + EM(TCP_CLOSE) \
> + EM(TCP_CLOSE_WAIT) \
> + EM(TCP_LAST_ACK)\
> + EM(TCP_LISTEN) \
> + EM(TCP_CLOSING) \
> + EMe(TCP_NEW_SYN_RECV)
> +
> +/* enums need to be exported to user space */
> +#undef EM
> +#undef EMe
> +#define EM(a) TRACE_DEFINE_ENUM(a);
> +#define EMe(a)TRACE_DEFINE_ENUM(a);
> +
> +tcp_state_names
> +
> +#undef EM
> +#undef EMe
> +#define EM(a) tcp_state_name(a),
> +#define EMe(a)tcp_state_name(a)
> +
> #define tcp_state_name(state) { state, #state }
> #define show_tcp_state_name(val) \
> - __print_symbolic(val, \
> - tcp_state_name(TCP_ESTABLISHED),\
> - tcp_state_name(TCP_SYN_SENT), \
> - tcp_state_name(TCP_SYN_RECV), \
> - tcp_state_name(TCP_FIN_WAIT1), \
> - tcp_state_name(TCP_FIN_WAIT2), \
> - tcp_state_name(TCP_TIME_WAIT), \
> - tcp_state_name(TCP_CLOSE), \
> - tcp_state_name(TCP_CLOSE_WAIT), \
> - tcp_state_name(TCP_LAST_ACK), \
> - tcp_state_name(TCP_LISTEN), \
> - tcp_state_name(TCP_CLOSING),\
> - tcp_state_name(TCP_NEW_SYN_RECV))
> + __print_symbolic(val, tcp_state_names)
>
> /*
> * tcp event with arguments sk and skb
> --
> 2.13.6
>
Reviewed-and-tested-by: Song Liu
Re: [PATCH] tcp: Export to userspace the TCP state names for the trace events
2017-11-10 8:57 GMT+08:00 Steven Rostedt :
>
> From: "Steven Rostedt (VMware)"
>
> The TCP trace events (specifically tcp_set_state), maps emums to symbol
> names via __print_symbolic(). But this only works for reading trace events
> from the tracefs trace files. If perf or trace-cmd were to record these
> events, the event format file does not convert the enum names into numbers,
> and you get something like:
>
> __print_symbolic(REC->oldstate,
> { TCP_ESTABLISHED, "TCP_ESTABLISHED" },
> { TCP_SYN_SENT, "TCP_SYN_SENT" },
> { TCP_SYN_RECV, "TCP_SYN_RECV" },
> { TCP_FIN_WAIT1, "TCP_FIN_WAIT1" },
> { TCP_FIN_WAIT2, "TCP_FIN_WAIT2" },
> { TCP_TIME_WAIT, "TCP_TIME_WAIT" },
> { TCP_CLOSE, "TCP_CLOSE" },
> { TCP_CLOSE_WAIT, "TCP_CLOSE_WAIT" },
> { TCP_LAST_ACK, "TCP_LAST_ACK" },
> { TCP_LISTEN, "TCP_LISTEN" },
> { TCP_CLOSING, "TCP_CLOSING" },
> { TCP_NEW_SYN_RECV, "TCP_NEW_SYN_RECV" })
>
> Where trace-cmd and perf do not know the values of those enums.
>
> Use the TRACE_DEFINE_ENUM() macros that will have the trace events convert
> the enum strings into their values at system boot. This will allow perf and
> trace-cmd to see actual numbers and not enums:
>
> __print_symbolic(REC->oldstate,
> { 1, "TCP_ESTABLISHED" },
> { 2, "TCP_SYN_SENT" },
> { 3, "TCP_SYN_RECV" },
> { 4, "TCP_FIN_WAIT1" },
> { 5, "TCP_FIN_WAIT2" },
> { 6, "TCP_TIME_WAIT" },
> { 7, "TCP_CLOSE" },
> { 8, "TCP_CLOSE_WAIT" },
> { 9, "TCP_LAST_ACK" },
> { 10, "TCP_LISTEN" },
> { 11, "TCP_CLOSING" },
> { 12, "TCP_NEW_SYN_RECV" })
>
> Signed-off-by: Steven Rostedt (VMware)
> ---
> include/trace/events/tcp.h | 41 -
> 1 file changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
> index 07a6cbf1..62e5bad7901f 100644
> --- a/include/trace/events/tcp.h
> +++ b/include/trace/events/tcp.h
> @@ -9,21 +9,36 @@
> #include
> #include
>
> +#define tcp_state_names\
> + EM(TCP_ESTABLISHED) \
> + EM(TCP_SYN_SENT)\
> + EM(TCP_SYN_RECV)\
> + EM(TCP_FIN_WAIT1) \
> + EM(TCP_FIN_WAIT2) \
> + EM(TCP_TIME_WAIT) \
> + EM(TCP_CLOSE) \
> + EM(TCP_CLOSE_WAIT) \
> + EM(TCP_LAST_ACK)\
> + EM(TCP_LISTEN) \
> + EM(TCP_CLOSING) \
> + EMe(TCP_NEW_SYN_RECV)
> +
> +/* enums need to be exported to user space */
> +#undef EM
> +#undef EMe
> +#define EM(a) TRACE_DEFINE_ENUM(a);
> +#define EMe(a)TRACE_DEFINE_ENUM(a);
> +
> +tcp_state_names
> +
> +#undef EM
> +#undef EMe
> +#define EM(a) tcp_state_name(a),
> +#define EMe(a)tcp_state_name(a)
> +
> #define tcp_state_name(state) { state, #state }
> #define show_tcp_state_name(val) \
> - __print_symbolic(val, \
> - tcp_state_name(TCP_ESTABLISHED),\
> - tcp_state_name(TCP_SYN_SENT), \
> - tcp_state_name(TCP_SYN_RECV), \
> - tcp_state_name(TCP_FIN_WAIT1), \
> - tcp_state_name(TCP_FIN_WAIT2), \
> - tcp_state_name(TCP_TIME_WAIT), \
> - tcp_state_name(TCP_CLOSE), \
> - tcp_state_name(TCP_CLOSE_WAIT), \
> - tcp_state_name(TCP_LAST_ACK), \
> - tcp_state_name(TCP_LISTEN), \
> - tcp_state_name(TCP_CLOSING),\
> - tcp_state_name(TCP_NEW_SYN_RECV))
> + __print_symbolic(val, tcp_state_names)
>
> /*
> * tcp event with arguments sk and skb
> --
> 2.13.6
>
Could the macro tcp_state_name() be renamed ？
If is included in include/net/tcp.h, it will
cause compile error, because there's another function tcp_state_name()
defined in net/netfilter/ipvs/ip_vs_proto_tcp.c.
static const char * tcp_state_name(int state)
{
if (state >= IP_VS_TCP_S_LAST)
return "ERR!";
return tcp_state_name_table[state] ? tcp_state_name_table[state] : "?";
}
Thanks
Yafang
Re: [PATCH] tcp: Export to userspace the TCP state names for the trace events
On Fri, 10 Nov 2017 12:56:06 +0800
Yafang Shao wrote:
> Could the macro tcp_state_name() be renamed ？
> If is included in include/net/tcp.h, it will
Ideally, you don't want to include trace/events/*.h headers in other
headers, as they can have side effects if those headers are included in
other trace/events/*.h headers.
> cause compile error, because there's another function tcp_state_name()
> defined in net/netfilter/ipvs/ip_vs_proto_tcp.c.
> static const char * tcp_state_name(int state)
> {
>
> if (state >= IP_VS_TCP_S_LAST)
>
> return "ERR!";
>
> return tcp_state_name_table[state] ? tcp_state_name_table[state] :
> "?";
>
> }
But that said, I didn't make up the trace_state_name(), it was already
there in net-next before this patch.
But yeah, in actuality, I would have just done:
#define EM(a) { a, #a },
#define EMe(a) { a, #a }
directly. Which we can still do.
-- Steve
Re: [PATCH] tcp: Export to userspace the TCP state names for the trace events
> On Nov 10, 2017, at 7:07 AM, Steven Rostedt wrote:
>
> On Fri, 10 Nov 2017 12:56:06 +0800
> Yafang Shao wrote:
>
>> Could the macro tcp_state_name() be renamed ？
>> If is included in include/net/tcp.h, it will
>
> Ideally, you don't want to include trace/events/*.h headers in other
> headers, as they can have side effects if those headers are included in
> other trace/events/*.h headers.
>
>> cause compile error, because there's another function tcp_state_name()
>> defined in net/netfilter/ipvs/ip_vs_proto_tcp.c.
>>static const char * tcp_state_name(int state)
>>{
>>
>>if (state >= IP_VS_TCP_S_LAST)
>>
>>return "ERR!";
>>
>>return tcp_state_name_table[state] ? tcp_state_name_table[state] :
>> "?";
>>
>>}
>
> But that said, I didn't make up the trace_state_name(), it was already
> there in net-next before this patch.
>
> But yeah, in actuality, I would have just done:
>
> #define EM(a) { a, #a },
> #define EMe(a){ a, #a }
>
> directly. Which we can still do.
>
> -- Steve
>
How about we undef tcp_state_name and tcp_event_names at the end of
include/trace/events/tcp.h?
Thanks,
Song
Re: [PATCH] tcp: Export to userspace the TCP state names for the trace events
2017-11-10 15:07 GMT+00:00 Steven Rostedt :
> On Fri, 10 Nov 2017 12:56:06 +0800
> Yafang Shao wrote:
>
>> Could the macro tcp_state_name() be renamed ？
>> If is included in include/net/tcp.h, it will
>
> Ideally, you don't want to include trace/events/*.h headers in other
> headers, as they can have side effects if those headers are included in
> other trace/events/*.h headers.
>
Actually I find trace/events/*.h is included in lots of other headers,
for example,
net/rxrpc/ar-internal.h
include/linux/bpf_trace.h
fs/f2fs/trace.h
fs/afs/internal.h
arch/x86/include/asm/mmu_context.h
...
Are these files doing properly ?
Should we fix them ?
But per my understanding, it is ok to include trace/events/*.h in
other headers because we defined TRACE_SYSTEM as well, as a
consequence those headers should not included in trace/events/*.h. If
that happens, it may means that one of the these two TRACE_SYSTEM is
not defined properly. Maybe these two TRACE_SYSTEM should be merged to
one TRACE_SYSTEM.
>> cause compile error, because there's another function tcp_state_name()
>> defined in net/netfilter/ipvs/ip_vs_proto_tcp.c.
>> static const char * tcp_state_name(int state)
>> {
>>
>> if (state >= IP_VS_TCP_S_LAST)
>>
>> return "ERR!";
>>
>> return tcp_state_name_table[state] ? tcp_state_name_table[state] :
>> "?";
>>
>> }
>
> But that said, I didn't make up the trace_state_name(), it was already
> there in net-next before this patch.
>
I know that is not your fault.
But as you are modifying this file, it is better to modify it in your
patch as well.
So we need not submit another new patch to fix it.
> But yeah, in actuality, I would have just done:
>
> #define EM(a) { a, #a },
> #define EMe(a) { a, #a }
>
> directly. Which we can still do.
>
> -- Steve
>
The suggestion from Song is good to fix it.
Thanks
Yafang
Re: [PATCH] tcp: Export to userspace the TCP state names for the trace events
On Sat, 11 Nov 2017 02:06:00 +
Yafang Shao wrote:
> 2017-11-10 15:07 GMT+00:00 Steven Rostedt :
> > On Fri, 10 Nov 2017 12:56:06 +0800
> > Yafang Shao wrote:
> >
> >> Could the macro tcp_state_name() be renamed ？
> >> If is included in include/net/tcp.h, it will
> >
> > Ideally, you don't want to include trace/events/*.h headers in other
> > headers, as they can have side effects if those headers are included in
> > other trace/events/*.h headers.
> >
>
> Actually I find trace/events/*.h is included in lots of other headers,
> for example,
>
> net/rxrpc/ar-internal.h
This is an internal header, so it's not that likely to be used where it
shouldn't be.
> include/linux/bpf_trace.h
> fs/f2fs/trace.h
The above two are actually headers specifically used to pull in the
trace/events/*.h headers.
> fs/afs/internal.h
another internal header. Unlikely to be misused.
> arch/x86/include/asm/mmu_context.h
This one, hmm, probably should be fixed.
> ...
>
> Are these files doing properly ?
Most yes, some probably not.
> Should we fix them ?
Probably, but if they are used incorrectly, it would usually fail on
build (The same global functions and variables would be defined).
>
> But per my understanding, it is ok to include trace/events/*.h in
> other headers because we defined TRACE_SYSTEM as well, as a
> consequence those headers should not included in trace/events/*.h. If
> that happens, it may means that one of the these two TRACE_SYSTEM is
> not defined properly. Maybe these two TRACE_SYSTEM should be merged to
> one TRACE_SYSTEM.
Two different files may have the same TRACE_SYSTEM defined. That's not
an issue.
The issue is, if you have a trace/events/*.h header in a popular file
(like it use to be in include/linux/slab.h), then it can cause issues
if another trace/events/*.h header includes it. That's because each
trace/events/*.h header must be included with CREATE_TRACE_POINTS only
once.
>
>
> >> cause compile error, because there's another function tcp_state_name()
> >> defined in net/netfilter/ipvs/ip_vs_proto_tcp.c.
> >> static const char * tcp_state_name(int state)
> >> {
> >>
> >> if (state >= IP_VS_TCP_S_LAST)
> >>
> >> return "ERR!";
> >>
> >> return tcp_state_name_table[state] ? tcp_state_name_table[state] :
> >> "?";
> >>
> >> }
> >
> > But that said, I didn't make up the trace_state_name(), it was already
> > there in net-next before this patch.
> >
>
> I know that is not your fault.
:-)
> But as you are modifying this file, it is better to modify it in your
> patch as well.
> So we need not submit another new patch to fix it.
I could whip up a patch 2.
>
> > But yeah, in actuality, I would have just done:
> >
> > #define EM(a) { a, #a },
> > #define EMe(a) { a, #a }
> >
> > directly. Which we can still do.
> >
> > -- Steve
> >
>
> The suggestion from Song is good to fix it.
Song's suggestion seems like it can simple be a patch added on top of
mine. As it is somewhat agnostic to the fix I'm making. That is, it's a
different problem, and thus should be a different patch.
-- Steve
Re: [PATCH] tcp: Export to userspace the TCP state names for the trace events
2017-11-11 3:32 GMT+00:00 Steven Rostedt :
> On Sat, 11 Nov 2017 02:06:00 +
> Yafang Shao wrote:
>
>> 2017-11-10 15:07 GMT+00:00 Steven Rostedt :
>> > On Fri, 10 Nov 2017 12:56:06 +0800
>> > Yafang Shao wrote:
>> >
>> >> Could the macro tcp_state_name() be renamed ？
>> >> If is included in include/net/tcp.h, it will
>> >
>> > Ideally, you don't want to include trace/events/*.h headers in other
>> > headers, as they can have side effects if those headers are included in
>> > other trace/events/*.h headers.
>> >
>>
>> Actually I find trace/events/*.h is included in lots of other headers,
>> for example,
>>
>> net/rxrpc/ar-internal.h
>
> This is an internal header, so it's not that likely to be used where it
> shouldn't be.
>
>> include/linux/bpf_trace.h
>> fs/f2fs/trace.h
>
> The above two are actually headers specifically used to pull in the
> trace/events/*.h headers.
>
>> fs/afs/internal.h
>
> another internal header. Unlikely to be misused.
>
>> arch/x86/include/asm/mmu_context.h
>
> This one, hmm, probably should be fixed.
>
>> ...
>>
>> Are these files doing properly ?
>
> Most yes, some probably not.
>
>> Should we fix them ?
>
> Probably, but if they are used incorrectly, it would usually fail on
> build (The same global functions and variables would be defined).
>
>>
>> But per my understanding, it is ok to include trace/events/*.h in
>> other headers because we defined TRACE_SYSTEM as well, as a
>> consequence those headers should not included in trace/events/*.h. If
>> that happens, it may means that one of the these two TRACE_SYSTEM is
>> not defined properly. Maybe these two TRACE_SYSTEM should be merged to
>> one TRACE_SYSTEM.
>
> Two different files may have the same TRACE_SYSTEM defined. That's not
> an issue.
>
> The issue is, if you have a trace/events/*.h header in a popular file
> (like it use to be in include/linux/slab.h), then it can cause issues
> if another trace/events/*.h header includes it. That's because each
> trace/events/*.h header must be included with CREATE_TRACE_POINTS only
> once.
>
Understood.
Thanks for explanation.
>>
>>
>> >> cause compile error, because there's another function tcp_state_name()
>> >> defined in net/netfilter/ipvs/ip_vs_proto_tcp.c.
>> >> static const char * tcp_state_name(int state)
>> >> {
>> >>
>> >> if (state >= IP_VS_TCP_S_LAST)
>> >>
>> >> return "ERR!";
>> >>
>> >> return tcp_state_name_table[state] ? tcp_state_name_table[state]
>> >> : "?";
>> >>
>> >> }
>> >
>> > But that said, I didn't make up the trace_state_name(), it was already
>> > there in net-next before this patch.
>> >
>>
>> I know that is not your fault.
>
> :-)
>
>> But as you are modifying this file, it is better to modify it in your
>> patch as well.
>> So we need not submit another new patch to fix it.
>
> I could whip up a patch 2.
>
>>
>> > But yeah, in actuality, I would have just done:
>> >
>> > #define EM(a) { a, #a },
>> > #define EMe(a) { a, #a }
>> >
>> > directly. Which we can still do.
>> >
>> > -- Steve
>> >
>>
>> The suggestion from Song is good to fix it.
>
> Song's suggestion seems like it can simple be a patch added on top of
> mine. As it is somewhat agnostic to the fix I'm making. That is, it's a
> different problem, and thus should be a different patch.
>
Got it. These two issues should be fixed in two different patches :-)
Thanks
Yafang
