Re: [PATCH v3] perf hists browser: Consolidate callchain print functions in TUI

2014-08-22 Thread Arnaldo Carvalho de Melo 
Em Fri, Aug 22, 2014 at 09:13:21AM +0900, Namhyung Kim escreveu:
> Currently there're two callchain print functions in TUI - one for the
> hists browser and another for file dump.  They do almost same job so
> it'd be better consolidate the codes.
> 
> To do that, provide two callbacks to the generic logic - one for
> printing and another for checking whether it should stop.

Thanks a lot, I just changed the name of the check_output_full_fn
parameter to "is_output_full", as "check" is way too generic to help one
understand what is it that is being checked.

I also aligned aligned the members of the struct callchain_print_arg, as
suggested by Ingo.

Applied.

- Arnaldo
 
> Cc: Frederic Weisbecker 
> Signed-off-by: Namhyung Kim 
> ---
>  tools/perf/ui/browsers/hists.c | 203 
> -
>  1 file changed, 80 insertions(+), 123 deletions(-)
> 
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 519353d9f5fb..026421e0d53d 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -477,20 +477,37 @@ static char *callchain_list__sym_name(struct 
> callchain_list *cl,
>   return bf;
>  }
>  
> +struct callchain_print_arg {
> + /* for hists browser */
> + off_t row_offset;
> + bool is_current_entry;
> +
> + /* for file dump */
> + FILE *fp;
> + int printed;
> +};
> +
> +typedef void (*print_callchain_entry_fn)(struct hist_browser *browser,
> +  struct callchain_list *chain,
> +  const char *str, int offset,
> +  unsigned short row,
> +  struct callchain_print_arg *arg);
> +
>  static void hist_browser__show_callchain_entry(struct hist_browser *browser,
>  struct callchain_list *chain,
> -unsigned short row, int offset,
> -char folded_sign, const char 
> *str,
> -bool *is_current_entry)
> +const char *str, int offset,
> +unsigned short row,
> +struct callchain_print_arg *arg)
>  {
>   int color, width;
> + char folded_sign = callchain_list__folded(chain);
>  
>   color = HE_COLORSET_NORMAL;
>   width = browser->b.width - (offset + 2);
>   if (ui_browser__is_current_entry(>b, row)) {
>   browser->selection = >ms;
>   color = HE_COLORSET_SELECTED;
> - *is_current_entry = true;
> + arg->is_current_entry = true;
>   }
>  
>   ui_browser__set_color(>b, color);
> @@ -500,12 +517,41 @@ static void hist_browser__show_callchain_entry(struct 
> hist_browser *browser,
>   slsmg_write_nstring(str, width);
>  }
>  
> +static void hist_browser__fprintf_callchain_entry(struct hist_browser *b 
> __maybe_unused,
> +   struct callchain_list *chain,
> +   const char *str, int offset,
> +   unsigned short row 
> __maybe_unused,
> +   struct callchain_print_arg 
> *arg)
> +{
> + char folded_sign = callchain_list__folded(chain);
> +
> + arg->printed += fprintf(arg->fp, "%*s%c %s\n", offset, " ",
> + folded_sign, str);
> +}
> +
> +typedef bool (*check_output_full_fn)(struct hist_browser *browser,
> +  unsigned short row);
> +
> +static bool hist_browser__check_output_full(struct hist_browser *browser,
> + unsigned short row)
> +{
> + return browser->b.rows == row;
> +}
> +
> +static bool hist_browser__check_dump_full(struct hist_browser *browser 
> __maybe_unused,
> +   unsigned short row __maybe_unused)
> +{
> + return false;
> +}
> +
>  #define LEVEL_OFFSET_STEP 3
>  
>  static int hist_browser__show_callchain(struct hist_browser *browser,
>   struct rb_root *root, int level,
> - unsigned short row, off_t *row_offset,
> - u64 total, bool *is_current_entry)
> + unsigned short row, u64 total,
> + print_callchain_entry_fn print,
> + struct callchain_print_arg *arg,
> + check_output_full_fn check)
>  {
>   struct rb_node *node;
>   int first_row = row, offset = level * LEVEL_OFFSET_STEP;
> @@ -532,8 +578,8 @@ static int hist_browser__show_callchain(struct 
> hist_browser *browser,
>

Re: [PATCH v3] perf hists browser: Consolidate callchain print functions in TUI

2014-08-22 Thread Arnaldo Carvalho de Melo 
Em Fri, Aug 22, 2014 at 09:13:21AM +0900, Namhyung Kim escreveu:
 Currently there're two callchain print functions in TUI - one for the
 hists browser and another for file dump.  They do almost same job so
 it'd be better consolidate the codes.
 
 To do that, provide two callbacks to the generic logic - one for
 printing and another for checking whether it should stop.

Thanks a lot, I just changed the name of the check_output_full_fn
parameter to is_output_full, as check is way too generic to help one
understand what is it that is being checked.

I also aligned aligned the members of the struct callchain_print_arg, as
suggested by Ingo.

Applied.

- Arnaldo
 
 Cc: Frederic Weisbecker fweis...@gmail.com
 Signed-off-by: Namhyung Kim namhy...@kernel.org
 ---
  tools/perf/ui/browsers/hists.c | 203 
 -
  1 file changed, 80 insertions(+), 123 deletions(-)
 
 diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
 index 519353d9f5fb..026421e0d53d 100644
 --- a/tools/perf/ui/browsers/hists.c
 +++ b/tools/perf/ui/browsers/hists.c
 @@ -477,20 +477,37 @@ static char *callchain_list__sym_name(struct 
 callchain_list *cl,
   return bf;
  }
  
 +struct callchain_print_arg {
 + /* for hists browser */
 + off_t row_offset;
 + bool is_current_entry;
 +
 + /* for file dump */
 + FILE *fp;
 + int printed;
 +};
 +
 +typedef void (*print_callchain_entry_fn)(struct hist_browser *browser,
 +  struct callchain_list *chain,
 +  const char *str, int offset,
 +  unsigned short row,
 +  struct callchain_print_arg *arg);
 +
  static void hist_browser__show_callchain_entry(struct hist_browser *browser,
  struct callchain_list *chain,
 -unsigned short row, int offset,
 -char folded_sign, const char 
 *str,
 -bool *is_current_entry)
 +const char *str, int offset,
 +unsigned short row,
 +struct callchain_print_arg *arg)
  {
   int color, width;
 + char folded_sign = callchain_list__folded(chain);
  
   color = HE_COLORSET_NORMAL;
   width = browser-b.width - (offset + 2);
   if (ui_browser__is_current_entry(browser-b, row)) {
   browser-selection = chain-ms;
   color = HE_COLORSET_SELECTED;
 - *is_current_entry = true;
 + arg-is_current_entry = true;
   }
  
   ui_browser__set_color(browser-b, color);
 @@ -500,12 +517,41 @@ static void hist_browser__show_callchain_entry(struct 
 hist_browser *browser,
   slsmg_write_nstring(str, width);
  }
  
 +static void hist_browser__fprintf_callchain_entry(struct hist_browser *b 
 __maybe_unused,
 +   struct callchain_list *chain,
 +   const char *str, int offset,
 +   unsigned short row 
 __maybe_unused,
 +   struct callchain_print_arg 
 *arg)
 +{
 + char folded_sign = callchain_list__folded(chain);
 +
 + arg-printed += fprintf(arg-fp, %*s%c %s\n, offset,  ,
 + folded_sign, str);
 +}
 +
 +typedef bool (*check_output_full_fn)(struct hist_browser *browser,
 +  unsigned short row);
 +
 +static bool hist_browser__check_output_full(struct hist_browser *browser,
 + unsigned short row)
 +{
 + return browser-b.rows == row;
 +}
 +
 +static bool hist_browser__check_dump_full(struct hist_browser *browser 
 __maybe_unused,
 +   unsigned short row __maybe_unused)
 +{
 + return false;
 +}
 +
  #define LEVEL_OFFSET_STEP 3
  
  static int hist_browser__show_callchain(struct hist_browser *browser,
   struct rb_root *root, int level,
 - unsigned short row, off_t *row_offset,
 - u64 total, bool *is_current_entry)
 + unsigned short row, u64 total,
 + print_callchain_entry_fn print,
 + struct callchain_print_arg *arg,
 + check_output_full_fn check)
  {
   struct rb_node *node;
   int first_row = row, offset = level * LEVEL_OFFSET_STEP;
 @@ -532,8 +578,8 @@ static int hist_browser__show_callchain(struct 
 hist_browser *browser,
   extra_offset = LEVEL_OFFSET_STEP;
  
   folded_sign =

[PATCH v3] perf hists browser: Consolidate callchain print functions in TUI

2014-08-21 Thread Namhyung Kim 
Currently there're two callchain print functions in TUI - one for the
hists browser and another for file dump.  They do almost same job so
it'd be better consolidate the codes.

To do that, provide two callbacks to the generic logic - one for
printing and another for checking whether it should stop.

Cc: Frederic Weisbecker 
Signed-off-by: Namhyung Kim 
---
 tools/perf/ui/browsers/hists.c | 203 -
 1 file changed, 80 insertions(+), 123 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 519353d9f5fb..026421e0d53d 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -477,20 +477,37 @@ static char *callchain_list__sym_name(struct 
callchain_list *cl,
return bf;
 }
 
+struct callchain_print_arg {
+   /* for hists browser */
+   off_t row_offset;
+   bool is_current_entry;
+
+   /* for file dump */
+   FILE *fp;
+   int printed;
+};
+
+typedef void (*print_callchain_entry_fn)(struct hist_browser *browser,
+struct callchain_list *chain,
+const char *str, int offset,
+unsigned short row,
+struct callchain_print_arg *arg);
+
 static void hist_browser__show_callchain_entry(struct hist_browser *browser,
   struct callchain_list *chain,
-  unsigned short row, int offset,
-  char folded_sign, const char 
*str,
-  bool *is_current_entry)
+  const char *str, int offset,
+  unsigned short row,
+  struct callchain_print_arg *arg)
 {
int color, width;
+   char folded_sign = callchain_list__folded(chain);
 
color = HE_COLORSET_NORMAL;
width = browser->b.width - (offset + 2);
if (ui_browser__is_current_entry(>b, row)) {
browser->selection = >ms;
color = HE_COLORSET_SELECTED;
-   *is_current_entry = true;
+   arg->is_current_entry = true;
}
 
ui_browser__set_color(>b, color);
@@ -500,12 +517,41 @@ static void hist_browser__show_callchain_entry(struct 
hist_browser *browser,
slsmg_write_nstring(str, width);
 }
 
+static void hist_browser__fprintf_callchain_entry(struct hist_browser *b 
__maybe_unused,
+ struct callchain_list *chain,
+ const char *str, int offset,
+ unsigned short row 
__maybe_unused,
+ struct callchain_print_arg 
*arg)
+{
+   char folded_sign = callchain_list__folded(chain);
+
+   arg->printed += fprintf(arg->fp, "%*s%c %s\n", offset, " ",
+   folded_sign, str);
+}
+
+typedef bool (*check_output_full_fn)(struct hist_browser *browser,
+unsigned short row);
+
+static bool hist_browser__check_output_full(struct hist_browser *browser,
+   unsigned short row)
+{
+   return browser->b.rows == row;
+}
+
+static bool hist_browser__check_dump_full(struct hist_browser *browser 
__maybe_unused,
+ unsigned short row __maybe_unused)
+{
+   return false;
+}
+
 #define LEVEL_OFFSET_STEP 3
 
 static int hist_browser__show_callchain(struct hist_browser *browser,
struct rb_root *root, int level,
-   unsigned short row, off_t *row_offset,
-   u64 total, bool *is_current_entry)
+   unsigned short row, u64 total,
+   print_callchain_entry_fn print,
+   struct callchain_print_arg *arg,
+   check_output_full_fn check)
 {
struct rb_node *node;
int first_row = row, offset = level * LEVEL_OFFSET_STEP;
@@ -532,8 +578,8 @@ static int hist_browser__show_callchain(struct hist_browser 
*browser,
extra_offset = LEVEL_OFFSET_STEP;
 
folded_sign = callchain_list__folded(chain);
-   if (*row_offset != 0) {
-   --*row_offset;
+   if (arg->row_offset != 0) {
+   arg->row_offset--;
goto do_next;
}
 
@@ -550,13 +596,11 @@ static int hist_browser__show_callchain(struct 
hist_browser *browser,

[PATCH v3] perf hists browser: Consolidate callchain print functions in TUI

2014-08-21 Thread Namhyung Kim 
Currently there're two callchain print functions in TUI - one for the
hists browser and another for file dump.  They do almost same job so
it'd be better consolidate the codes.

To do that, provide two callbacks to the generic logic - one for
printing and another for checking whether it should stop.

Cc: Frederic Weisbecker fweis...@gmail.com
Signed-off-by: Namhyung Kim namhy...@kernel.org
---
 tools/perf/ui/browsers/hists.c | 203 -
 1 file changed, 80 insertions(+), 123 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 519353d9f5fb..026421e0d53d 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -477,20 +477,37 @@ static char *callchain_list__sym_name(struct 
callchain_list *cl,
return bf;
 }
 
+struct callchain_print_arg {
+   /* for hists browser */
+   off_t row_offset;
+   bool is_current_entry;
+
+   /* for file dump */
+   FILE *fp;
+   int printed;
+};
+
+typedef void (*print_callchain_entry_fn)(struct hist_browser *browser,
+struct callchain_list *chain,
+const char *str, int offset,
+unsigned short row,
+struct callchain_print_arg *arg);
+
 static void hist_browser__show_callchain_entry(struct hist_browser *browser,
   struct callchain_list *chain,
-  unsigned short row, int offset,
-  char folded_sign, const char 
*str,
-  bool *is_current_entry)
+  const char *str, int offset,
+  unsigned short row,
+  struct callchain_print_arg *arg)
 {
int color, width;
+   char folded_sign = callchain_list__folded(chain);
 
color = HE_COLORSET_NORMAL;
width = browser-b.width - (offset + 2);
if (ui_browser__is_current_entry(browser-b, row)) {
browser-selection = chain-ms;
color = HE_COLORSET_SELECTED;
-   *is_current_entry = true;
+   arg-is_current_entry = true;
}
 
ui_browser__set_color(browser-b, color);
@@ -500,12 +517,41 @@ static void hist_browser__show_callchain_entry(struct 
hist_browser *browser,
slsmg_write_nstring(str, width);
 }
 
+static void hist_browser__fprintf_callchain_entry(struct hist_browser *b 
__maybe_unused,
+ struct callchain_list *chain,
+ const char *str, int offset,
+ unsigned short row 
__maybe_unused,
+ struct callchain_print_arg 
*arg)
+{
+   char folded_sign = callchain_list__folded(chain);
+
+   arg-printed += fprintf(arg-fp, %*s%c %s\n, offset,  ,
+   folded_sign, str);
+}
+
+typedef bool (*check_output_full_fn)(struct hist_browser *browser,
+unsigned short row);
+
+static bool hist_browser__check_output_full(struct hist_browser *browser,
+   unsigned short row)
+{
+   return browser-b.rows == row;
+}
+
+static bool hist_browser__check_dump_full(struct hist_browser *browser 
__maybe_unused,
+ unsigned short row __maybe_unused)
+{
+   return false;
+}
+
 #define LEVEL_OFFSET_STEP 3
 
 static int hist_browser__show_callchain(struct hist_browser *browser,
struct rb_root *root, int level,
-   unsigned short row, off_t *row_offset,
-   u64 total, bool *is_current_entry)
+   unsigned short row, u64 total,
+   print_callchain_entry_fn print,
+   struct callchain_print_arg *arg,
+   check_output_full_fn check)
 {
struct rb_node *node;
int first_row = row, offset = level * LEVEL_OFFSET_STEP;
@@ -532,8 +578,8 @@ static int hist_browser__show_callchain(struct hist_browser 
*browser,
extra_offset = LEVEL_OFFSET_STEP;
 
folded_sign = callchain_list__folded(chain);
-   if (*row_offset != 0) {
-   --*row_offset;
+   if (arg-row_offset != 0) {
+   arg-row_offset--;
goto do_next;
}
 
@@ -550,13 +596,11 @@ static int hist_browser__show_callchain(struct