From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751843AbdBAILC (ORCPT ); Wed, 1 Feb 2017 03:11:02 -0500 Received: from www62.your-server.de ([213.133.104.62]:39265 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751590AbdBAIK7 (ORCPT ); Wed, 1 Feb 2017 03:10:59 -0500 Message-ID: <5891980C.9020203@iogearbox.net> Date: Wed, 01 Feb 2017 09:10:52 +0100 From: Daniel Borkmann User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Steven Rostedt CC: davem@davemloft.net, ast@kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Arnaldo Carvalho de Melo Subject: Re: [PATCH net-next 1/3] trace: add variant without spacing in trace_print_hex_seq References: <20170130154425.581bf631@gandalf.local.home> In-Reply-To: <20170130154425.581bf631@gandalf.local.home> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Authenticated-Sender: daniel@iogearbox.net Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/30/2017 09:44 PM, Steven Rostedt wrote: > On Wed, 25 Jan 2017 02:28:16 +0100 > Daniel Borkmann wrote: > >> For upcoming tracepoint support for BPF, we want to dump the program's >> tag. Format should be similar to __print_hex(), but without spacing. >> Add a __print_hex_str() variant for exactly that purpose that reuses >> trace_print_hex_seq(). >> >> Signed-off-by: Daniel Borkmann >> Cc: Steven Rostedt >> Cc: Arnaldo Carvalho de Melo >> --- >> include/linux/trace_events.h | 3 ++- >> include/trace/trace_events.h | 8 +++++++- >> kernel/trace/trace_output.c | 7 ++++--- >> 3 files changed, 13 insertions(+), 5 deletions(-) >> >> diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h >> index be00761..cfa475a 100644 >> --- a/include/linux/trace_events.h >> +++ b/include/linux/trace_events.h >> @@ -33,7 +33,8 @@ const char *trace_print_bitmask_seq(struct trace_seq *p, void *bitmask_ptr, >> unsigned int bitmask_size); >> >> const char *trace_print_hex_seq(struct trace_seq *p, >> - const unsigned char *buf, int len); >> + const unsigned char *buf, int len, >> + bool spacing); > > Hmm, "spacing" doesn't really mean much. What about the invert of it, > and have "concatenate"? Sure, I'm fine with that. >> const char *trace_print_array_seq(struct trace_seq *p, >> const void *buf, int count, >> diff --git a/include/trace/trace_events.h b/include/trace/trace_events.h >> index 467e12f..9f68462 100644 >> --- a/include/trace/trace_events.h >> +++ b/include/trace/trace_events.h >> @@ -297,7 +297,12 @@ >> #endif >> >> #undef __print_hex >> -#define __print_hex(buf, buf_len) trace_print_hex_seq(p, buf, buf_len) >> +#define __print_hex(buf, buf_len) \ >> + trace_print_hex_seq(p, buf, buf_len, true) >> + >> +#undef __print_hex_str >> +#define __print_hex_str(buf, buf_len) \ >> + trace_print_hex_seq(p, buf, buf_len, false) >> >> #undef __print_array >> #define __print_array(array, count, el_size) \ >> @@ -711,6 +716,7 @@ >> #undef __print_flags >> #undef __print_symbolic >> #undef __print_hex >> +#undef __print_hex_str >> #undef __get_dynamic_array >> #undef __get_dynamic_array_len >> #undef __get_str >> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c >> index 5d33a73..30a144b1 100644 >> --- a/kernel/trace/trace_output.c >> +++ b/kernel/trace/trace_output.c >> @@ -163,14 +163,15 @@ enum print_line_t trace_print_printk_msg_only(struct trace_iterator *iter) >> EXPORT_SYMBOL_GPL(trace_print_bitmask_seq); > > With the addition of this boolean parameter, this function shold > probably have a kernel doc header, that can explain the parameters. Yeah, I can add that. Since the patch got already applied, I will send a follow-up for adding the kdoc and for changing the bool logic/name as concatenate. Thanks for your review! Daniel