On Thu, 24 Sep 2020 10:06:09 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@xxxxxxxxx> wrote:
> There are internal library functions, which are not decalred as a static.
> They are used inside the library from different files. Hide them from
> the library users, as they are not part of the API.
> These functions are made hidden and are renamed without the prefix "tep_":
> tep_free_plugin_paths
> tep_peek_char
> tep_buffer_init
> tep_get_input_buf_ptr
> tep_get_input_buf
> tep_read_token
> tep_free_token
> tep_free_event
> tep_free_format_field
You have tep_free_plugin_paths twice.
> tep_free_plugin_paths
>
> Reported-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@xxxxxxxxx>
> ---
> tools/lib/traceevent/event-parse-api.c | 8 +--
> tools/lib/traceevent/event-parse-local.h | 24 +++++--
> tools/lib/traceevent/event-parse.c | 88 ++++++------------------
> tools/lib/traceevent/event-parse.h | 8 ---
> tools/lib/traceevent/event-plugin.c | 2 +-
> tools/lib/traceevent/parse-filter.c | 23 +++----
> 6 files changed, 52 insertions(+), 101 deletions(-)
>
> diff --git a/tools/lib/traceevent/event-parse-api.c b/tools/lib/traceevent/event-parse-api.c
> index 4faf52a65791..23c4a00ae45b 100644
> --- a/tools/lib/traceevent/event-parse-api.c
> +++ b/tools/lib/traceevent/event-parse-api.c
> @@ -92,7 +92,7 @@ bool tep_test_flag(struct tep_handle *tep, enum tep_flag flag)
> return false;
> }
>
> -unsigned short tep_data2host2(struct tep_handle *tep, unsigned short data)
> +__hidden unsigned short _data2host2(struct tep_handle *tep, unsigned short data)
> {
> unsigned short swap;
>
> @@ -105,7 +105,7 @@ unsigned short tep_data2host2(struct tep_handle *tep, unsigned short data)
> return swap;
> }
>
> -unsigned int tep_data2host4(struct tep_handle *tep, unsigned int data)
> +__hidden unsigned int _data2host4(struct tep_handle *tep, unsigned int data)
As these are hidden, we don't need the leading underscore. Thus
data2host2 and data2host4, etc, is fine.
> {
> unsigned int swap;
>
> @@ -120,8 +120,8 @@ unsigned int tep_data2host4(struct tep_handle *tep, unsigned int data)
> return swap;
> }
>
> -unsigned long long
> -tep_data2host8(struct tep_handle *tep, unsigned long long data)
> +__hidden unsigned long long
> +_data2host8(struct tep_handle *tep, unsigned long long data)
> {
> unsigned long long swap;
>
> diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h
> index d805a920af6f..d9417437679a 100644
> --- a/tools/lib/traceevent/event-parse-local.h
> +++ b/tools/lib/traceevent/event-parse-local.h
> @@ -15,6 +15,8 @@ struct event_handler;
> struct func_resolver;
> struct tep_plugins_dir;
>
> +#define __hidden __attribute__((visibility ("hidden")))
> +
> struct tep_handle {
> int ref_count;
>
> @@ -102,12 +104,20 @@ struct tep_print_parse {
> struct tep_print_arg *len_as_arg;
> };
>
> -void tep_free_event(struct tep_event *event);
> -void tep_free_format_field(struct tep_format_field *field);
> -void tep_free_plugin_paths(struct tep_handle *tep);
> -
> -unsigned short tep_data2host2(struct tep_handle *tep, unsigned short data);
> -unsigned int tep_data2host4(struct tep_handle *tep, unsigned int data);
> -unsigned long long tep_data2host8(struct tep_handle *tep, unsigned long long data);
> +void free_tep_event(struct tep_event *event);
> +void free_tep_format_field(struct tep_format_field *field);
> +void free_tep_plugin_paths(struct tep_handle *tep);
> +
> +unsigned short _data2host2(struct tep_handle *tep, unsigned short data);
> +unsigned int _data2host4(struct tep_handle *tep, unsigned int data);
> +unsigned long long _data2host8(struct tep_handle *tep, unsigned long long data);
> +
> +/* access to the internal parser */
> +int __peek_char(void);
Same for __peek_char(). We can call it peek_char().
Other than that, I think this looks good.
When sending the next version, I would send it to Arnaldo directly and
Cc this list, LKML, and myself. Then I'll just ack it for Arnaldo to
take it directly from you.
Add v2 in the subject '[PATCH v2]' and state the differences of v1
after the '---' of your signed off by. You could also add,
V1 found here: https://lore.kernel.org/r/20200924070609.100771-2-tz.stoyanov@xxxxxxxxx
so that Arnaldo knows where to find v1, as I don't think he's on this
list.
Thanks!
-- Steve
|