[PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end

[PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end

[Namhyung Kim]

Hi,

This is a patch set for improving GTK2 support. Patch 1 - 5 are
generalization of setup_browser() to support GTK browser seamlessly.
These are basically resend of previous version that got ack from
Pekka - Only patch 4 was slightly modified to make TUI default as
it does now.

On patch 6, introduce perf_error_ops structure and its helper
perf_error__[un]register_functions to set UI specific error
reporting functions and make TUI use them. To do this, split
TUI specific bits to ui/tui/util.c file. Patch 7 applies it
to GTK also.

Patches are based on current tip/perf/core - 1fa2e84db3f9 ("Merge tag
'perf-annotate-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux
into perf/core"). All of these are build-tested for all combinations
of NO_NEWT and NO_GTK2.

Any comments are welcome.

Thanks,
Namhyung


Namhyung Kim (7):
  perf ui: Make setup_browser() generic
  perf ui: Drop arg[cv] arguments from perf_gtk_setup_browser()
  perf ui/gtk: Rename functions for consistency
  perf ui: Add gtk2 support into setup_browser()
  perf ui: Change fallback policy of setup_browser()
  perf ui: Introduce struct perf_error_ops
  perf ui/gtk: Use struct perf_error_ops

 tools/perf/Makefile         |    9 ++
 tools/perf/builtin-report.c |   10 +-
 tools/perf/ui/gtk/browser.c |   37 +++---
 tools/perf/ui/gtk/gtk.h     |    4 +
 tools/perf/ui/gtk/setup.c   |   18 +++
 tools/perf/ui/gtk/util.c    |   67 +++++++++++
 tools/perf/ui/setup.c       |  168 +++++---------------------
 tools/perf/ui/tui/setup.c   |  146 +++++++++++++++++++++++
 tools/perf/ui/tui/util.c    |  243 +++++++++++++++++++++++++++++++++++++
 tools/perf/ui/util.c        |  277 +++++++++----------------------------------
 tools/perf/ui/util.h        |    9 +-
 tools/perf/util/cache.h     |   24 ++--
 tools/perf/util/debug.c     |    2 +-
 tools/perf/util/debug.h     |   23 +++-
 14 files changed, 635 insertions(+), 402 deletions(-)
 create mode 100644 tools/perf/ui/gtk/setup.c
 create mode 100644 tools/perf/ui/gtk/util.c
 create mode 100644 tools/perf/ui/tui/setup.c
 create mode 100644 tools/perf/ui/tui/util.c

-- 
1.7.10

[PATCH 1/7] perf ui: Make setup_browser() generic

[Namhyung Kim]

The setup_browser contained newt-related codes in it.
As gtk front-end added recently, it should be more
generic to handle both cases properly. So move newt
codes to the ui__init() for now.

Acked-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/ui/setup.c |   44 +++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index 85a69faa09aa..becdcd0d9ce7 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -93,14 +93,37 @@ static void newt_suspend(void *d __used)
 	newtResume();
 }
 
+static void ui__exit(void);
+
+static void ui__signal(int sig)
+{
+	ui__exit();
+	psignal(sig, "perf");
+	exit(0);
+}
+
 static int ui__init(void)
 {
-	int err = SLkp_init();
+	int err;
 
-	if (err < 0)
+	newtInit();
+	err = SLkp_init();
+	if (err < 0) {
+		pr_err("TUI initialization failed.\n");
 		goto out;
+	}
 
 	SLkp_define_keysym((char *)"^(kB)", SL_KEY_UNTAB);
+
+	newtSetSuspendCallback(newt_suspend, NULL);
+	ui_helpline__init();
+	ui_browser__init();
+
+	signal(SIGSEGV, ui__signal);
+	signal(SIGFPE, ui__signal);
+	signal(SIGINT, ui__signal);
+	signal(SIGQUIT, ui__signal);
+	signal(SIGTERM, ui__signal);
 out:
 	return err;
 }
@@ -113,13 +136,6 @@ static void ui__exit(void)
 	SLang_reset_tty();
 }
 
-static void ui__signal(int sig)
-{
-	ui__exit();
-	psignal(sig, "perf");
-	exit(0);
-}
-
 void setup_browser(bool fallback_to_pager)
 {
 	if (!isatty(1) || !use_browser || dump_trace) {
@@ -130,17 +146,7 @@ void setup_browser(bool fallback_to_pager)
 	}
 
 	use_browser = 1;
-	newtInit();
 	ui__init();
-	newtSetSuspendCallback(newt_suspend, NULL);
-	ui_helpline__init();
-	ui_browser__init();
-
-	signal(SIGSEGV, ui__signal);
-	signal(SIGFPE, ui__signal);
-	signal(SIGINT, ui__signal);
-	signal(SIGQUIT, ui__signal);
-	signal(SIGTERM, ui__signal);
 }
 
 void exit_browser(bool wait_for_ok)
-- 
1.7.10

[tip:perf/core] perf ui: Make setup_browser() generic

[tip-bot for Namhyung Kim]

Commit-ID:  ca09b2e1b307724666577859eb460ac6d4c67330
Gitweb:     http://git.kernel.org/tip/ca09b2e1b307724666577859eb460ac6d4c67330
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 30 Apr 2012 13:55:05 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 May 2012 16:17:20 -0300

perf ui: Make setup_browser() generic

The setup_browser contained newt-related codes in it.

As gtk front-end added recently, it should be more generic to handle
both cases properly.

So move newt codes to the ui__init() for now.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Acked-by: Pekka Enberg <penberg@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1335761711-31403-2-git-send-email-namhyung.kim@lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/setup.c |   44 +++++++++++++++++++++++++-------------------
 1 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index 85a69fa..becdcd0 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -93,14 +93,37 @@ static void newt_suspend(void *d __used)
 	newtResume();
 }
 
+static void ui__exit(void);
+
+static void ui__signal(int sig)
+{
+	ui__exit();
+	psignal(sig, "perf");
+	exit(0);
+}
+
 static int ui__init(void)
 {
-	int err = SLkp_init();
+	int err;
 
-	if (err < 0)
+	newtInit();
+	err = SLkp_init();
+	if (err < 0) {
+		pr_err("TUI initialization failed.\n");
 		goto out;
+	}
 
 	SLkp_define_keysym((char *)"^(kB)", SL_KEY_UNTAB);
+
+	newtSetSuspendCallback(newt_suspend, NULL);
+	ui_helpline__init();
+	ui_browser__init();
+
+	signal(SIGSEGV, ui__signal);
+	signal(SIGFPE, ui__signal);
+	signal(SIGINT, ui__signal);
+	signal(SIGQUIT, ui__signal);
+	signal(SIGTERM, ui__signal);
 out:
 	return err;
 }
@@ -113,13 +136,6 @@ static void ui__exit(void)
 	SLang_reset_tty();
 }
 
-static void ui__signal(int sig)
-{
-	ui__exit();
-	psignal(sig, "perf");
-	exit(0);
-}
-
 void setup_browser(bool fallback_to_pager)
 {
 	if (!isatty(1) || !use_browser || dump_trace) {
@@ -130,17 +146,7 @@ void setup_browser(bool fallback_to_pager)
 	}
 
 	use_browser = 1;
-	newtInit();
 	ui__init();
-	newtSetSuspendCallback(newt_suspend, NULL);
-	ui_helpline__init();
-	ui_browser__init();
-
-	signal(SIGSEGV, ui__signal);
-	signal(SIGFPE, ui__signal);
-	signal(SIGINT, ui__signal);
-	signal(SIGQUIT, ui__signal);
-	signal(SIGTERM, ui__signal);
 }
 
 void exit_browser(bool wait_for_ok)

[PATCH 2/7] perf ui: Drop arg[cv] arguments from perf_gtk_setup_browser()

[Namhyung Kim]

As perf doesn't allow to specify gtk command-line option,
drop the arguments and pass NULL to gtk_init(). This makes
the function easier to be called from setup_browser().

Acked-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/builtin-report.c |    2 +-
 tools/perf/ui/gtk/browser.c |    5 ++---
 tools/perf/util/cache.h     |    4 ++--
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index cec2b8cee80c..2b20001848f5 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -678,7 +678,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
 
 	if (strcmp(report.input_name, "-") != 0) {
 		if (report.use_gtk)
-			perf_gtk_setup_browser(argc, argv, true);
+			perf_gtk_setup_browser(true);
 		else
 			setup_browser(true);
 	} else {
diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index 258352a2356c..a1a83de3f459 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -9,10 +9,9 @@
 
 #define MAX_COLUMNS			32
 
-void perf_gtk_setup_browser(int argc, const char *argv[],
-			    bool fallback_to_pager __used)
+void perf_gtk_setup_browser(bool fallback_to_pager __used)
 {
-	gtk_init(&argc, (char ***)&argv);
+	gtk_init(NULL, NULL);
 }
 
 void perf_gtk_exit_browser(bool wait_for_ok __used)
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 8dd224df3e54..d22ca689fb15 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -46,14 +46,14 @@ void exit_browser(bool wait_for_ok);
 #endif
 
 #ifdef NO_GTK2_SUPPORT
-static inline void perf_gtk_setup_browser(int argc __used, const char *argv[] __used, bool fallback_to_pager)
+static inline void perf_gtk_setup_browser(bool fallback_to_pager)
 {
 	if (fallback_to_pager)
 		setup_pager();
 }
 static inline void perf_gtk_exit_browser(bool wait_for_ok __used) {}
 #else
-void perf_gtk_setup_browser(int argc, const char *argv[], bool fallback_to_pager);
+void perf_gtk_setup_browser(bool fallback_to_pager);
 void perf_gtk_exit_browser(bool wait_for_ok);
 #endif
 
-- 
1.7.10

[tip:perf/core] perf ui gtk: Drop arg[cv] arguments from perf_gtk_setup_browser()

[tip-bot for Namhyung Kim]

Commit-ID:  7706f966323f32f3ea13121b5918851432876ae5
Gitweb:     http://git.kernel.org/tip/7706f966323f32f3ea13121b5918851432876ae5
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 30 Apr 2012 13:55:06 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 May 2012 16:17:25 -0300

perf ui gtk: Drop arg[cv] arguments from perf_gtk_setup_browser()

As perf doesn't allow to specify gtk command-line option, drop the
arguments and pass NULL to gtk_init().

This makes the function easier to be called from setup_browser().

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Acked-by: Pekka Enberg <penberg@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1335761711-31403-3-git-send-email-namhyung.kim@lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-report.c |    2 +-
 tools/perf/ui/gtk/browser.c |    5 ++---
 tools/perf/util/cache.h     |    4 ++--
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index cec2b8c..2b20001 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -678,7 +678,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
 
 	if (strcmp(report.input_name, "-") != 0) {
 		if (report.use_gtk)
-			perf_gtk_setup_browser(argc, argv, true);
+			perf_gtk_setup_browser(true);
 		else
 			setup_browser(true);
 	} else {
diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index 258352a..a1a83de 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -9,10 +9,9 @@
 
 #define MAX_COLUMNS			32
 
-void perf_gtk_setup_browser(int argc, const char *argv[],
-			    bool fallback_to_pager __used)
+void perf_gtk_setup_browser(bool fallback_to_pager __used)
 {
-	gtk_init(&argc, (char ***)&argv);
+	gtk_init(NULL, NULL);
 }
 
 void perf_gtk_exit_browser(bool wait_for_ok __used)
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 8dd224d..d22ca68 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -46,14 +46,14 @@ void exit_browser(bool wait_for_ok);
 #endif
 
 #ifdef NO_GTK2_SUPPORT
-static inline void perf_gtk_setup_browser(int argc __used, const char *argv[] __used, bool fallback_to_pager)
+static inline void perf_gtk_setup_browser(bool fallback_to_pager)
 {
 	if (fallback_to_pager)
 		setup_pager();
 }
 static inline void perf_gtk_exit_browser(bool wait_for_ok __used) {}
 #else
-void perf_gtk_setup_browser(int argc, const char *argv[], bool fallback_to_pager);
+void perf_gtk_setup_browser(bool fallback_to_pager);
 void perf_gtk_exit_browser(bool wait_for_ok);
 #endif
 

[PATCH 3/7] perf ui/gtk: Rename functions for consistency

[Namhyung Kim]

We use double underscore characters to distinguish its subsystem
and actual function name.

Acked-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/builtin-report.c |    2 +-
 tools/perf/ui/gtk/browser.c |   24 ++++++++++++------------
 tools/perf/util/cache.h     |    8 ++++----
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 2b20001848f5..06115ffaa0b4 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -678,7 +678,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
 
 	if (strcmp(report.input_name, "-") != 0) {
 		if (report.use_gtk)
-			perf_gtk_setup_browser(true);
+			perf_gtk__setup_browser(true);
 		else
 			setup_browser(true);
 	} else {
diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index a1a83de3f459..5eafd9b3b783 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -9,23 +9,23 @@
 
 #define MAX_COLUMNS			32
 
-void perf_gtk_setup_browser(bool fallback_to_pager __used)
+void perf_gtk__setup_browser(bool fallback_to_pager __used)
 {
 	gtk_init(NULL, NULL);
 }
 
-void perf_gtk_exit_browser(bool wait_for_ok __used)
+void perf_gtk__exit_browser(bool wait_for_ok __used)
 {
 	gtk_main_quit();
 }
 
-static void perf_gtk_signal(int sig)
+static void perf_gtk__signal(int sig)
 {
 	psignal(sig, "perf");
 	gtk_main_quit();
 }
 
-static void perf_gtk_resize_window(GtkWidget *window)
+static void perf_gtk__resize_window(GtkWidget *window)
 {
 	GdkRectangle rect;
 	GdkScreen *screen;
@@ -45,7 +45,7 @@ static void perf_gtk_resize_window(GtkWidget *window)
 	gtk_window_resize(GTK_WINDOW(window), width, height);
 }
 
-static void perf_gtk_show_hists(GtkWidget *window, struct hists *hists)
+static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 {
 	GType col_types[MAX_COLUMNS];
 	GtkCellRenderer *renderer;
@@ -141,11 +141,11 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 	GtkWidget *notebook;
 	GtkWidget *window;
 
-	signal(SIGSEGV, perf_gtk_signal);
-	signal(SIGFPE,  perf_gtk_signal);
-	signal(SIGINT,  perf_gtk_signal);
-	signal(SIGQUIT, perf_gtk_signal);
-	signal(SIGTERM, perf_gtk_signal);
+	signal(SIGSEGV, perf_gtk__signal);
+	signal(SIGFPE,  perf_gtk__signal);
+	signal(SIGINT,  perf_gtk__signal);
+	signal(SIGQUIT, perf_gtk__signal);
+	signal(SIGTERM, perf_gtk__signal);
 
 	window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
 
@@ -167,7 +167,7 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 							GTK_POLICY_AUTOMATIC,
 							GTK_POLICY_AUTOMATIC);
 
-		perf_gtk_show_hists(scrolled_window, hists);
+		perf_gtk__show_hists(scrolled_window, hists);
 
 		tab_label = gtk_label_new(evname);
 
@@ -178,7 +178,7 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 
 	gtk_widget_show_all(window);
 
-	perf_gtk_resize_window(window);
+	perf_gtk__resize_window(window);
 
 	gtk_window_set_position(GTK_WINDOW(window), GTK_WIN_POS_CENTER);
 
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index d22ca689fb15..3428b777396d 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -46,15 +46,15 @@ void exit_browser(bool wait_for_ok);
 #endif
 
 #ifdef NO_GTK2_SUPPORT
-static inline void perf_gtk_setup_browser(bool fallback_to_pager)
+static inline void perf_gtk__setup_browser(bool fallback_to_pager)
 {
 	if (fallback_to_pager)
 		setup_pager();
 }
-static inline void perf_gtk_exit_browser(bool wait_for_ok __used) {}
+static inline void perf_gtk__exit_browser(bool wait_for_ok __used) {}
 #else
-void perf_gtk_setup_browser(bool fallback_to_pager);
-void perf_gtk_exit_browser(bool wait_for_ok);
+void perf_gtk__setup_browser(bool fallback_to_pager);
+void perf_gtk__exit_browser(bool wait_for_ok);
 #endif
 
 char *alias_lookup(const char *alias);
-- 
1.7.10

[tip:perf/core] perf ui gtk: Rename functions for consistency

[tip-bot for Namhyung Kim]

Commit-ID:  28e62b90d95a4ed8ae2ba93879003665051581a6
Gitweb:     http://git.kernel.org/tip/28e62b90d95a4ed8ae2ba93879003665051581a6
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 30 Apr 2012 13:55:07 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 May 2012 16:17:28 -0300

perf ui gtk: Rename functions for consistency

We use double underscore characters to distinguish its subsystem and
actual function name.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Acked-by: Pekka Enberg <penberg@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1335761711-31403-4-git-send-email-namhyung.kim@lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-report.c |    2 +-
 tools/perf/ui/gtk/browser.c |   24 ++++++++++++------------
 tools/perf/util/cache.h     |    8 ++++----
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 2b20001..06115ff 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -678,7 +678,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
 
 	if (strcmp(report.input_name, "-") != 0) {
 		if (report.use_gtk)
-			perf_gtk_setup_browser(true);
+			perf_gtk__setup_browser(true);
 		else
 			setup_browser(true);
 	} else {
diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index a1a83de..5eafd9b 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -9,23 +9,23 @@
 
 #define MAX_COLUMNS			32
 
-void perf_gtk_setup_browser(bool fallback_to_pager __used)
+void perf_gtk__setup_browser(bool fallback_to_pager __used)
 {
 	gtk_init(NULL, NULL);
 }
 
-void perf_gtk_exit_browser(bool wait_for_ok __used)
+void perf_gtk__exit_browser(bool wait_for_ok __used)
 {
 	gtk_main_quit();
 }
 
-static void perf_gtk_signal(int sig)
+static void perf_gtk__signal(int sig)
 {
 	psignal(sig, "perf");
 	gtk_main_quit();
 }
 
-static void perf_gtk_resize_window(GtkWidget *window)
+static void perf_gtk__resize_window(GtkWidget *window)
 {
 	GdkRectangle rect;
 	GdkScreen *screen;
@@ -45,7 +45,7 @@ static void perf_gtk_resize_window(GtkWidget *window)
 	gtk_window_resize(GTK_WINDOW(window), width, height);
 }
 
-static void perf_gtk_show_hists(GtkWidget *window, struct hists *hists)
+static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 {
 	GType col_types[MAX_COLUMNS];
 	GtkCellRenderer *renderer;
@@ -141,11 +141,11 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 	GtkWidget *notebook;
 	GtkWidget *window;
 
-	signal(SIGSEGV, perf_gtk_signal);
-	signal(SIGFPE,  perf_gtk_signal);
-	signal(SIGINT,  perf_gtk_signal);
-	signal(SIGQUIT, perf_gtk_signal);
-	signal(SIGTERM, perf_gtk_signal);
+	signal(SIGSEGV, perf_gtk__signal);
+	signal(SIGFPE,  perf_gtk__signal);
+	signal(SIGINT,  perf_gtk__signal);
+	signal(SIGQUIT, perf_gtk__signal);
+	signal(SIGTERM, perf_gtk__signal);
 
 	window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
 
@@ -167,7 +167,7 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 							GTK_POLICY_AUTOMATIC,
 							GTK_POLICY_AUTOMATIC);
 
-		perf_gtk_show_hists(scrolled_window, hists);
+		perf_gtk__show_hists(scrolled_window, hists);
 
 		tab_label = gtk_label_new(evname);
 
@@ -178,7 +178,7 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 
 	gtk_widget_show_all(window);
 
-	perf_gtk_resize_window(window);
+	perf_gtk__resize_window(window);
 
 	gtk_window_set_position(GTK_WINDOW(window), GTK_WIN_POS_CENTER);
 
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index d22ca68..3428b77 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -46,15 +46,15 @@ void exit_browser(bool wait_for_ok);
 #endif
 
 #ifdef NO_GTK2_SUPPORT
-static inline void perf_gtk_setup_browser(bool fallback_to_pager)
+static inline void perf_gtk__setup_browser(bool fallback_to_pager)
 {
 	if (fallback_to_pager)
 		setup_pager();
 }
-static inline void perf_gtk_exit_browser(bool wait_for_ok __used) {}
+static inline void perf_gtk__exit_browser(bool wait_for_ok __used) {}
 #else
-void perf_gtk_setup_browser(bool fallback_to_pager);
-void perf_gtk_exit_browser(bool wait_for_ok);
+void perf_gtk__setup_browser(bool fallback_to_pager);
+void perf_gtk__exit_browser(bool wait_for_ok);
 #endif
 
 char *alias_lookup(const char *alias);

[PATCH 4/7] perf ui: Add gtk2 support into setup_browser()

[Namhyung Kim]

Now setup_browser can handle gtk2 front-end so split the
TUI code to ui/tui/setup.c in order to remove dependency.
To this end, make ui__init/exit global symbols and take
an argument. Also split gtk code to ui/gtk/setup.c.

Acked-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/Makefile         |    6 ++
 tools/perf/builtin-report.c |   10 +--
 tools/perf/ui/gtk/browser.c |   10 ---
 tools/perf/ui/gtk/setup.c   |   12 +++
 tools/perf/ui/setup.c       |  169 +++++++------------------------------------
 tools/perf/ui/tui/setup.c   |  140 +++++++++++++++++++++++++++++++++++
 tools/perf/util/cache.h     |   23 ++++--
 7 files changed, 205 insertions(+), 165 deletions(-)
 create mode 100644 tools/perf/ui/gtk/setup.c
 create mode 100644 tools/perf/ui/tui/setup.c

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index e98e14c88532..5fd1d60ed8a5 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -481,6 +481,7 @@ else
 		LIB_OBJS += $(OUTPUT)ui/helpline.o
 		LIB_OBJS += $(OUTPUT)ui/progress.o
 		LIB_OBJS += $(OUTPUT)ui/util.o
+		LIB_OBJS += $(OUTPUT)ui/tui/setup.o
 		LIB_H += ui/browser.h
 		LIB_H += ui/browsers/map.h
 		LIB_H += ui/helpline.h
@@ -503,6 +504,11 @@ else
 		BASIC_CFLAGS += $(shell pkg-config --cflags gtk+-2.0)
 		EXTLIBS += $(shell pkg-config --libs gtk+-2.0)
 		LIB_OBJS += $(OUTPUT)ui/gtk/browser.o
+		LIB_OBJS += $(OUTPUT)ui/gtk/setup.o
+		# Make sure that it'd be included only once.
+		ifneq ($(findstring -DNO_NEWT_SUPPORT,$(BASIC_CFLAGS)),)
+			LIB_OBJS += $(OUTPUT)ui/setup.o
+		endif
 	endif
 endif
 
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 06115ffaa0b4..5df829f5bbf4 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -676,14 +676,10 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
 
 	}
 
-	if (strcmp(report.input_name, "-") != 0) {
-		if (report.use_gtk)
-			perf_gtk__setup_browser(true);
-		else
-			setup_browser(true);
-	} else {
+	if (strcmp(report.input_name, "-") != 0)
+		setup_browser(true);
+	else
 		use_browser = 0;
-	}
 
 	/*
 	 * Only in the newt browser we are doing integrated annotation,
diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index 5eafd9b3b783..0656c381a89c 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -9,16 +9,6 @@
 
 #define MAX_COLUMNS			32
 
-void perf_gtk__setup_browser(bool fallback_to_pager __used)
-{
-	gtk_init(NULL, NULL);
-}
-
-void perf_gtk__exit_browser(bool wait_for_ok __used)
-{
-	gtk_main_quit();
-}
-
 static void perf_gtk__signal(int sig)
 {
 	psignal(sig, "perf");
diff --git a/tools/perf/ui/gtk/setup.c b/tools/perf/ui/gtk/setup.c
new file mode 100644
index 000000000000..8c3b573e863d
--- /dev/null
+++ b/tools/perf/ui/gtk/setup.c
@@ -0,0 +1,12 @@
+#include "gtk.h"
+#include "../../util/cache.h"
+
+void perf_gtk__init(bool fallback_to_pager __used)
+{
+	gtk_init(NULL, NULL);
+}
+
+void perf_gtk__exit(bool wait_for_ok __used)
+{
+	gtk_main_quit();
+}
diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index becdcd0d9ce7..98130e099a0d 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -1,161 +1,44 @@
-#include <newt.h>
-#include <signal.h>
-#include <stdbool.h>
-
 #include "../cache.h"
 #include "../debug.h"
-#include "browser.h"
-#include "helpline.h"
-#include "ui.h"
-#include "util.h"
-#include "libslang.h"
-#include "keysyms.h"
-
-pthread_mutex_t ui__lock = PTHREAD_MUTEX_INITIALIZER;
-
-static volatile int ui__need_resize;
-
-void ui__refresh_dimensions(bool force)
-{
-	if (force || ui__need_resize) {
-		ui__need_resize = 0;
-		pthread_mutex_lock(&ui__lock);
-		SLtt_get_screen_size();
-		SLsmg_reinit_smg();
-		pthread_mutex_unlock(&ui__lock);
-	}
-}
-
-static void ui__sigwinch(int sig __used)
-{
-	ui__need_resize = 1;
-}
-
-static void ui__setup_sigwinch(void)
-{
-	static bool done;
-
-	if (done)
-		return;
-
-	done = true;
-	pthread__unblock_sigwinch();
-	signal(SIGWINCH, ui__sigwinch);
-}
-
-int ui__getch(int delay_secs)
-{
-	struct timeval timeout, *ptimeout = delay_secs ? &timeout : NULL;
-	fd_set read_set;
-	int err, key;
-
-	ui__setup_sigwinch();
-
-	FD_ZERO(&read_set);
-	FD_SET(0, &read_set);
-
-	if (delay_secs) {
-		timeout.tv_sec = delay_secs;
-		timeout.tv_usec = 0;
-	}
-
-        err = select(1, &read_set, NULL, NULL, ptimeout);
-
-	if (err == 0)
-		return K_TIMER;
-
-	if (err == -1) {
-		if (errno == EINTR)
-			return K_RESIZE;
-		return K_ERROR;
-	}
 
-	key = SLang_getkey();
-	if (key != K_ESC)
-		return key;
 
-	FD_ZERO(&read_set);
-	FD_SET(0, &read_set);
-	timeout.tv_sec = 0;
-	timeout.tv_usec = 20;
-        err = select(1, &read_set, NULL, NULL, &timeout);
-	if (err == 0)
-		return K_ESC;
-
-	SLang_ungetkey(key);
-	return SLkp_getkey();
-}
-
-static void newt_suspend(void *d __used)
-{
-	newtSuspend();
-	raise(SIGTSTP);
-	newtResume();
-}
-
-static void ui__exit(void);
-
-static void ui__signal(int sig)
-{
-	ui__exit();
-	psignal(sig, "perf");
-	exit(0);
-}
-
-static int ui__init(void)
+void setup_browser(bool fallback_to_pager)
 {
-	int err;
-
-	newtInit();
-	err = SLkp_init();
-	if (err < 0) {
-		pr_err("TUI initialization failed.\n");
-		goto out;
-	}
-
-	SLkp_define_keysym((char *)"^(kB)", SL_KEY_UNTAB);
+	if (!isatty(1) || dump_trace)
+		use_browser = 0;
 
-	newtSetSuspendCallback(newt_suspend, NULL);
-	ui_helpline__init();
-	ui_browser__init();
+	/* default to TUI */
+	if (use_browser < 0)
+		use_browser = 1;
 
-	signal(SIGSEGV, ui__signal);
-	signal(SIGFPE, ui__signal);
-	signal(SIGINT, ui__signal);
-	signal(SIGQUIT, ui__signal);
-	signal(SIGTERM, ui__signal);
-out:
-	return err;
-}
+	switch (use_browser) {
+	case 2:
+		perf_gtk__init(fallback_to_pager);
+		break;
 
-static void ui__exit(void)
-{
-	SLtt_set_cursor_visibility(1);
-	SLsmg_refresh();
-	SLsmg_reset_smg();
-	SLang_reset_tty();
-}
+	case 1:
+		ui__init(fallback_to_pager);
+		break;
 
-void setup_browser(bool fallback_to_pager)
-{
-	if (!isatty(1) || !use_browser || dump_trace) {
-		use_browser = 0;
+	default:
 		if (fallback_to_pager)
 			setup_pager();
-		return;
+		break;
 	}
-
-	use_browser = 1;
-	ui__init();
 }
 
 void exit_browser(bool wait_for_ok)
 {
-	if (use_browser > 0) {
-		if (wait_for_ok)
-			ui__question_window("Fatal Error",
-					    ui_helpline__last_msg,
-					    "Press any key...", 0);
-		ui__exit();
+	switch (use_browser) {
+	case 2:
+		perf_gtk__exit(wait_for_ok);
+		break;
+
+	case 1:
+		ui__exit(wait_for_ok);
+		break;
+
+	default:
+		break;
 	}
 }
diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
new file mode 100644
index 000000000000..0194cea2ea0e
--- /dev/null
+++ b/tools/perf/ui/tui/setup.c
@@ -0,0 +1,140 @@
+#include <newt.h>
+#include <signal.h>
+#include <stdbool.h>
+
+#include "../../util/cache.h"
+#include "../../util/debug.h"
+#include "../browser.h"
+#include "../helpline.h"
+#include "../ui.h"
+#include "../util.h"
+#include "../libslang.h"
+#include "../keysyms.h"
+
+pthread_mutex_t ui__lock = PTHREAD_MUTEX_INITIALIZER;
+
+static volatile int ui__need_resize;
+
+void ui__refresh_dimensions(bool force)
+{
+	if (force || ui__need_resize) {
+		ui__need_resize = 0;
+		pthread_mutex_lock(&ui__lock);
+		SLtt_get_screen_size();
+		SLsmg_reinit_smg();
+		pthread_mutex_unlock(&ui__lock);
+	}
+}
+
+static void ui__sigwinch(int sig __used)
+{
+	ui__need_resize = 1;
+}
+
+static void ui__setup_sigwinch(void)
+{
+	static bool done;
+
+	if (done)
+		return;
+
+	done = true;
+	pthread__unblock_sigwinch();
+	signal(SIGWINCH, ui__sigwinch);
+}
+
+int ui__getch(int delay_secs)
+{
+	struct timeval timeout, *ptimeout = delay_secs ? &timeout : NULL;
+	fd_set read_set;
+	int err, key;
+
+	ui__setup_sigwinch();
+
+	FD_ZERO(&read_set);
+	FD_SET(0, &read_set);
+
+	if (delay_secs) {
+		timeout.tv_sec = delay_secs;
+		timeout.tv_usec = 0;
+	}
+
+        err = select(1, &read_set, NULL, NULL, ptimeout);
+
+	if (err == 0)
+		return K_TIMER;
+
+	if (err == -1) {
+		if (errno == EINTR)
+			return K_RESIZE;
+		return K_ERROR;
+	}
+
+	key = SLang_getkey();
+	if (key != K_ESC)
+		return key;
+
+	FD_ZERO(&read_set);
+	FD_SET(0, &read_set);
+	timeout.tv_sec = 0;
+	timeout.tv_usec = 20;
+        err = select(1, &read_set, NULL, NULL, &timeout);
+	if (err == 0)
+		return K_ESC;
+
+	SLang_ungetkey(key);
+	return SLkp_getkey();
+}
+
+static void newt_suspend(void *d __used)
+{
+	newtSuspend();
+	raise(SIGTSTP);
+	newtResume();
+}
+
+static void ui__signal(int sig)
+{
+	ui__exit(false);
+	psignal(sig, "perf");
+	exit(0);
+}
+
+int ui__init(bool fallback_to_pager __used)
+{
+	int err;
+
+	newtInit();
+	err = SLkp_init();
+	if (err < 0) {
+		pr_err("TUI initialization failed.\n");
+		goto out;
+	}
+
+	SLkp_define_keysym((char *)"^(kB)", SL_KEY_UNTAB);
+
+	newtSetSuspendCallback(newt_suspend, NULL);
+	ui_helpline__init();
+	ui_browser__init();
+
+	signal(SIGSEGV, ui__signal);
+	signal(SIGFPE, ui__signal);
+	signal(SIGINT, ui__signal);
+	signal(SIGQUIT, ui__signal);
+	signal(SIGTERM, ui__signal);
+out:
+	return err;
+}
+
+void ui__exit(bool wait_for_ok)
+{
+	if (wait_for_ok)
+		ui__question_window("Fatal Error",
+				    ui_helpline__last_msg,
+				    "Press any key...", 0);
+
+	SLtt_set_cursor_visibility(1);
+	SLsmg_refresh();
+	SLsmg_reset_smg();
+	SLang_reset_tty();
+}
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 3428b777396d..761d4e998101 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -33,7 +33,7 @@ extern int pager_use_color;
 
 extern int use_browser;
 
-#ifdef NO_NEWT_SUPPORT
+#if defined(NO_NEWT_SUPPORT) && defined(NO_GTK2_SUPPORT)
 static inline void setup_browser(bool fallback_to_pager)
 {
 	if (fallback_to_pager)
@@ -43,19 +43,32 @@ static inline void exit_browser(bool wait_for_ok __used) {}
 #else
 void setup_browser(bool fallback_to_pager);
 void exit_browser(bool wait_for_ok);
+
+#ifdef NO_NEWT_SUPPORT
+static inline int ui__init(bool fallback_to_pager)
+{
+	if (fallback_to_pager)
+		setup_pager();
+	return 0;
+}
+static inline void ui__exit(bool wait_for_ok __used) {}
+#else
+int ui__init(bool fallback_to_pager);
+void ui__exit(bool wait_for_ok);
 #endif
 
 #ifdef NO_GTK2_SUPPORT
-static inline void perf_gtk__setup_browser(bool fallback_to_pager)
+static inline void perf_gtk__init(bool fallback_to_pager)
 {
 	if (fallback_to_pager)
 		setup_pager();
 }
-static inline void perf_gtk__exit_browser(bool wait_for_ok __used) {}
+static inline void perf_gtk__exit(bool wait_for_ok __used) {}
 #else
-void perf_gtk__setup_browser(bool fallback_to_pager);
-void perf_gtk__exit_browser(bool wait_for_ok);
+void perf_gtk__init(bool fallback_to_pager);
+void perf_gtk__exit(bool wait_for_ok);
 #endif
+#endif /* NO_NEWT_SUPPORT && NO_GTK2_SUPPORT */
 
 char *alias_lookup(const char *alias);
 int split_cmdline(char *cmdline, const char ***argv);
-- 
1.7.10

[tip:perf/core] perf ui: Add gtk2 support into setup_browser()

[tip-bot for Namhyung Kim]

Commit-ID:  281ef544a8476f750b9f378593c42b3e8a0b8788
Gitweb:     http://git.kernel.org/tip/281ef544a8476f750b9f378593c42b3e8a0b8788
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 30 Apr 2012 13:55:08 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 May 2012 16:17:34 -0300

perf ui: Add gtk2 support into setup_browser()

Now setup_browser can handle gtk2 front-end so split the TUI code to
ui/tui/setup.c in order to remove dependency.

To this end, make ui__init/exit global symbols and take an argument.
Also split gtk code to ui/gtk/setup.c.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Acked-by: Pekka Enberg <penberg@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1335761711-31403-5-git-send-email-namhyung.kim@lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile             |    6 ++
 tools/perf/builtin-report.c     |   10 +--
 tools/perf/ui/gtk/browser.c     |   10 ---
 tools/perf/ui/gtk/setup.c       |   12 +++
 tools/perf/ui/setup.c           |  169 ++++++---------------------------------
 tools/perf/ui/{ => tui}/setup.c |   53 ++++---------
 tools/perf/util/cache.h         |   23 ++++-
 7 files changed, 81 insertions(+), 202 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 4122a66..4734f41 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -483,6 +483,7 @@ else
 		LIB_OBJS += $(OUTPUT)ui/helpline.o
 		LIB_OBJS += $(OUTPUT)ui/progress.o
 		LIB_OBJS += $(OUTPUT)ui/util.o
+		LIB_OBJS += $(OUTPUT)ui/tui/setup.o
 		LIB_H += ui/browser.h
 		LIB_H += ui/browsers/map.h
 		LIB_H += ui/helpline.h
@@ -505,6 +506,11 @@ else
 		BASIC_CFLAGS += $(shell pkg-config --cflags gtk+-2.0)
 		EXTLIBS += $(shell pkg-config --libs gtk+-2.0)
 		LIB_OBJS += $(OUTPUT)ui/gtk/browser.o
+		LIB_OBJS += $(OUTPUT)ui/gtk/setup.o
+		# Make sure that it'd be included only once.
+		ifneq ($(findstring -DNO_NEWT_SUPPORT,$(BASIC_CFLAGS)),)
+			LIB_OBJS += $(OUTPUT)ui/setup.o
+		endif
 	endif
 endif
 
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 06115ff..5df829f 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -676,14 +676,10 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
 
 	}
 
-	if (strcmp(report.input_name, "-") != 0) {
-		if (report.use_gtk)
-			perf_gtk__setup_browser(true);
-		else
-			setup_browser(true);
-	} else {
+	if (strcmp(report.input_name, "-") != 0)
+		setup_browser(true);
+	else
 		use_browser = 0;
-	}
 
 	/*
 	 * Only in the newt browser we are doing integrated annotation,
diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index 5eafd9b..0656c38 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -9,16 +9,6 @@
 
 #define MAX_COLUMNS			32
 
-void perf_gtk__setup_browser(bool fallback_to_pager __used)
-{
-	gtk_init(NULL, NULL);
-}
-
-void perf_gtk__exit_browser(bool wait_for_ok __used)
-{
-	gtk_main_quit();
-}
-
 static void perf_gtk__signal(int sig)
 {
 	psignal(sig, "perf");
diff --git a/tools/perf/ui/gtk/setup.c b/tools/perf/ui/gtk/setup.c
new file mode 100644
index 0000000..8c3b573
--- /dev/null
+++ b/tools/perf/ui/gtk/setup.c
@@ -0,0 +1,12 @@
+#include "gtk.h"
+#include "../../util/cache.h"
+
+void perf_gtk__init(bool fallback_to_pager __used)
+{
+	gtk_init(NULL, NULL);
+}
+
+void perf_gtk__exit(bool wait_for_ok __used)
+{
+	gtk_main_quit();
+}
diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index becdcd0..98130e0 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -1,161 +1,44 @@
-#include <newt.h>
-#include <signal.h>
-#include <stdbool.h>
-
 #include "../cache.h"
 #include "../debug.h"
-#include "browser.h"
-#include "helpline.h"
-#include "ui.h"
-#include "util.h"
-#include "libslang.h"
-#include "keysyms.h"
-
-pthread_mutex_t ui__lock = PTHREAD_MUTEX_INITIALIZER;
-
-static volatile int ui__need_resize;
-
-void ui__refresh_dimensions(bool force)
-{
-	if (force || ui__need_resize) {
-		ui__need_resize = 0;
-		pthread_mutex_lock(&ui__lock);
-		SLtt_get_screen_size();
-		SLsmg_reinit_smg();
-		pthread_mutex_unlock(&ui__lock);
-	}
-}
-
-static void ui__sigwinch(int sig __used)
-{
-	ui__need_resize = 1;
-}
-
-static void ui__setup_sigwinch(void)
-{
-	static bool done;
-
-	if (done)
-		return;
-
-	done = true;
-	pthread__unblock_sigwinch();
-	signal(SIGWINCH, ui__sigwinch);
-}
-
-int ui__getch(int delay_secs)
-{
-	struct timeval timeout, *ptimeout = delay_secs ? &timeout : NULL;
-	fd_set read_set;
-	int err, key;
-
-	ui__setup_sigwinch();
-
-	FD_ZERO(&read_set);
-	FD_SET(0, &read_set);
-
-	if (delay_secs) {
-		timeout.tv_sec = delay_secs;
-		timeout.tv_usec = 0;
-	}
-
-        err = select(1, &read_set, NULL, NULL, ptimeout);
-
-	if (err == 0)
-		return K_TIMER;
-
-	if (err == -1) {
-		if (errno == EINTR)
-			return K_RESIZE;
-		return K_ERROR;
-	}
 
-	key = SLang_getkey();
-	if (key != K_ESC)
-		return key;
 
-	FD_ZERO(&read_set);
-	FD_SET(0, &read_set);
-	timeout.tv_sec = 0;
-	timeout.tv_usec = 20;
-        err = select(1, &read_set, NULL, NULL, &timeout);
-	if (err == 0)
-		return K_ESC;
-
-	SLang_ungetkey(key);
-	return SLkp_getkey();
-}
-
-static void newt_suspend(void *d __used)
-{
-	newtSuspend();
-	raise(SIGTSTP);
-	newtResume();
-}
-
-static void ui__exit(void);
-
-static void ui__signal(int sig)
-{
-	ui__exit();
-	psignal(sig, "perf");
-	exit(0);
-}
-
-static int ui__init(void)
+void setup_browser(bool fallback_to_pager)
 {
-	int err;
-
-	newtInit();
-	err = SLkp_init();
-	if (err < 0) {
-		pr_err("TUI initialization failed.\n");
-		goto out;
-	}
-
-	SLkp_define_keysym((char *)"^(kB)", SL_KEY_UNTAB);
+	if (!isatty(1) || dump_trace)
+		use_browser = 0;
 
-	newtSetSuspendCallback(newt_suspend, NULL);
-	ui_helpline__init();
-	ui_browser__init();
+	/* default to TUI */
+	if (use_browser < 0)
+		use_browser = 1;
 
-	signal(SIGSEGV, ui__signal);
-	signal(SIGFPE, ui__signal);
-	signal(SIGINT, ui__signal);
-	signal(SIGQUIT, ui__signal);
-	signal(SIGTERM, ui__signal);
-out:
-	return err;
-}
+	switch (use_browser) {
+	case 2:
+		perf_gtk__init(fallback_to_pager);
+		break;
 
-static void ui__exit(void)
-{
-	SLtt_set_cursor_visibility(1);
-	SLsmg_refresh();
-	SLsmg_reset_smg();
-	SLang_reset_tty();
-}
+	case 1:
+		ui__init(fallback_to_pager);
+		break;
 
-void setup_browser(bool fallback_to_pager)
-{
-	if (!isatty(1) || !use_browser || dump_trace) {
-		use_browser = 0;
+	default:
 		if (fallback_to_pager)
 			setup_pager();
-		return;
+		break;
 	}
-
-	use_browser = 1;
-	ui__init();
 }
 
 void exit_browser(bool wait_for_ok)
 {
-	if (use_browser > 0) {
-		if (wait_for_ok)
-			ui__question_window("Fatal Error",
-					    ui_helpline__last_msg,
-					    "Press any key...", 0);
-		ui__exit();
+	switch (use_browser) {
+	case 2:
+		perf_gtk__exit(wait_for_ok);
+		break;
+
+	case 1:
+		ui__exit(wait_for_ok);
+		break;
+
+	default:
+		break;
 	}
 }
diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/tui/setup.c
similarity index 75%
copy from tools/perf/ui/setup.c
copy to tools/perf/ui/tui/setup.c
index becdcd0..0194cea 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -2,14 +2,14 @@
 #include <signal.h>
 #include <stdbool.h>
 
-#include "../cache.h"
-#include "../debug.h"
-#include "browser.h"
-#include "helpline.h"
-#include "ui.h"
-#include "util.h"
-#include "libslang.h"
-#include "keysyms.h"
+#include "../../util/cache.h"
+#include "../../util/debug.h"
+#include "../browser.h"
+#include "../helpline.h"
+#include "../ui.h"
+#include "../util.h"
+#include "../libslang.h"
+#include "../keysyms.h"
 
 pthread_mutex_t ui__lock = PTHREAD_MUTEX_INITIALIZER;
 
@@ -93,16 +93,14 @@ static void newt_suspend(void *d __used)
 	newtResume();
 }
 
-static void ui__exit(void);
-
 static void ui__signal(int sig)
 {
-	ui__exit();
+	ui__exit(false);
 	psignal(sig, "perf");
 	exit(0);
 }
 
-static int ui__init(void)
+int ui__init(bool fallback_to_pager __used)
 {
 	int err;
 
@@ -128,34 +126,15 @@ out:
 	return err;
 }
 
-static void ui__exit(void)
+void ui__exit(bool wait_for_ok)
 {
+	if (wait_for_ok)
+		ui__question_window("Fatal Error",
+				    ui_helpline__last_msg,
+				    "Press any key...", 0);
+
 	SLtt_set_cursor_visibility(1);
 	SLsmg_refresh();
 	SLsmg_reset_smg();
 	SLang_reset_tty();
 }
-
-void setup_browser(bool fallback_to_pager)
-{
-	if (!isatty(1) || !use_browser || dump_trace) {
-		use_browser = 0;
-		if (fallback_to_pager)
-			setup_pager();
-		return;
-	}
-
-	use_browser = 1;
-	ui__init();
-}
-
-void exit_browser(bool wait_for_ok)
-{
-	if (use_browser > 0) {
-		if (wait_for_ok)
-			ui__question_window("Fatal Error",
-					    ui_helpline__last_msg,
-					    "Press any key...", 0);
-		ui__exit();
-	}
-}
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 3428b77..761d4e9 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -33,7 +33,7 @@ extern int pager_use_color;
 
 extern int use_browser;
 
-#ifdef NO_NEWT_SUPPORT
+#if defined(NO_NEWT_SUPPORT) && defined(NO_GTK2_SUPPORT)
 static inline void setup_browser(bool fallback_to_pager)
 {
 	if (fallback_to_pager)
@@ -43,19 +43,32 @@ static inline void exit_browser(bool wait_for_ok __used) {}
 #else
 void setup_browser(bool fallback_to_pager);
 void exit_browser(bool wait_for_ok);
+
+#ifdef NO_NEWT_SUPPORT
+static inline int ui__init(bool fallback_to_pager)
+{
+	if (fallback_to_pager)
+		setup_pager();
+	return 0;
+}
+static inline void ui__exit(bool wait_for_ok __used) {}
+#else
+int ui__init(bool fallback_to_pager);
+void ui__exit(bool wait_for_ok);
 #endif
 
 #ifdef NO_GTK2_SUPPORT
-static inline void perf_gtk__setup_browser(bool fallback_to_pager)
+static inline void perf_gtk__init(bool fallback_to_pager)
 {
 	if (fallback_to_pager)
 		setup_pager();
 }
-static inline void perf_gtk__exit_browser(bool wait_for_ok __used) {}
+static inline void perf_gtk__exit(bool wait_for_ok __used) {}
 #else
-void perf_gtk__setup_browser(bool fallback_to_pager);
-void perf_gtk__exit_browser(bool wait_for_ok);
+void perf_gtk__init(bool fallback_to_pager);
+void perf_gtk__exit(bool wait_for_ok);
 #endif
+#endif /* NO_NEWT_SUPPORT && NO_GTK2_SUPPORT */
 
 char *alias_lookup(const char *alias);
 int split_cmdline(char *cmdline, const char ***argv);

[PATCH 5/7] perf ui: Change fallback policy of setup_browser()

[Namhyung Kim]

If gtk2 support is not enabled (or failed for some reason) try TUI
again instead of falling directly back to the stdio interface.

Acked-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/ui/gtk/setup.c |    4 ++--
 tools/perf/ui/setup.c     |   13 +++++++------
 tools/perf/ui/tui/setup.c |    2 +-
 tools/perf/util/cache.h   |   15 ++++++---------
 4 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/tools/perf/ui/gtk/setup.c b/tools/perf/ui/gtk/setup.c
index 8c3b573e863d..829529957766 100644
--- a/tools/perf/ui/gtk/setup.c
+++ b/tools/perf/ui/gtk/setup.c
@@ -1,9 +1,9 @@
 #include "gtk.h"
 #include "../../util/cache.h"
 
-void perf_gtk__init(bool fallback_to_pager __used)
+int perf_gtk__init(void)
 {
-	gtk_init(NULL, NULL);
+	return gtk_init_check(NULL, NULL) ? 0 : -1;
 }
 
 void perf_gtk__exit(bool wait_for_ok __used)
diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index 98130e099a0d..9f5f888f73e3 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -13,13 +13,14 @@ void setup_browser(bool fallback_to_pager)
 
 	switch (use_browser) {
 	case 2:
-		perf_gtk__init(fallback_to_pager);
-		break;
-
+		if (perf_gtk__init() == 0)
+			break;
+		/* fall through */
 	case 1:
-		ui__init(fallback_to_pager);
-		break;
-
+		use_browser = 1;
+		if (ui__init() == 0)
+			break;
+		/* fall through */
 	default:
 		if (fallback_to_pager)
 			setup_pager();
diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index 0194cea2ea0e..d33e943ac434 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -100,7 +100,7 @@ static void ui__signal(int sig)
 	exit(0);
 }
 
-int ui__init(bool fallback_to_pager __used)
+int ui__init(void)
 {
 	int err;
 
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 761d4e998101..cff18c617d13 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -45,27 +45,24 @@ void setup_browser(bool fallback_to_pager);
 void exit_browser(bool wait_for_ok);
 
 #ifdef NO_NEWT_SUPPORT
-static inline int ui__init(bool fallback_to_pager)
+static inline int ui__init(void)
 {
-	if (fallback_to_pager)
-		setup_pager();
-	return 0;
+	return -1;
 }
 static inline void ui__exit(bool wait_for_ok __used) {}
 #else
-int ui__init(bool fallback_to_pager);
+int ui__init(void);
 void ui__exit(bool wait_for_ok);
 #endif
 
 #ifdef NO_GTK2_SUPPORT
-static inline void perf_gtk__init(bool fallback_to_pager)
+static inline int perf_gtk__init(void)
 {
-	if (fallback_to_pager)
-		setup_pager();
+	return -1;
 }
 static inline void perf_gtk__exit(bool wait_for_ok __used) {}
 #else
-void perf_gtk__init(bool fallback_to_pager);
+int perf_gtk__init(void);
 void perf_gtk__exit(bool wait_for_ok);
 #endif
 #endif /* NO_NEWT_SUPPORT && NO_GTK2_SUPPORT */
-- 
1.7.10

[tip:perf/core] perf ui: Change fallback policy of setup_browser()

[tip-bot for Namhyung Kim]

Commit-ID:  dc41b9b8f02dbe2228ae787d525dac43beebb7fa
Gitweb:     http://git.kernel.org/tip/dc41b9b8f02dbe2228ae787d525dac43beebb7fa
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 30 Apr 2012 13:55:09 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 May 2012 16:17:37 -0300

perf ui: Change fallback policy of setup_browser()

If gtk2 support is not enabled (or failed for some reason) try TUI again
instead of falling directly back to the stdio interface.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Acked-by: Pekka Enberg <penberg@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1335761711-31403-6-git-send-email-namhyung.kim@lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/gtk/setup.c |    4 ++--
 tools/perf/ui/setup.c     |   13 +++++++------
 tools/perf/ui/tui/setup.c |    2 +-
 tools/perf/util/cache.h   |   15 ++++++---------
 4 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/tools/perf/ui/gtk/setup.c b/tools/perf/ui/gtk/setup.c
index 8c3b573..8295299 100644
--- a/tools/perf/ui/gtk/setup.c
+++ b/tools/perf/ui/gtk/setup.c
@@ -1,9 +1,9 @@
 #include "gtk.h"
 #include "../../util/cache.h"
 
-void perf_gtk__init(bool fallback_to_pager __used)
+int perf_gtk__init(void)
 {
-	gtk_init(NULL, NULL);
+	return gtk_init_check(NULL, NULL) ? 0 : -1;
 }
 
 void perf_gtk__exit(bool wait_for_ok __used)
diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index 98130e0..9f5f888 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -13,13 +13,14 @@ void setup_browser(bool fallback_to_pager)
 
 	switch (use_browser) {
 	case 2:
-		perf_gtk__init(fallback_to_pager);
-		break;
-
+		if (perf_gtk__init() == 0)
+			break;
+		/* fall through */
 	case 1:
-		ui__init(fallback_to_pager);
-		break;
-
+		use_browser = 1;
+		if (ui__init() == 0)
+			break;
+		/* fall through */
 	default:
 		if (fallback_to_pager)
 			setup_pager();
diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index 0194cea..d33e943 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -100,7 +100,7 @@ static void ui__signal(int sig)
 	exit(0);
 }
 
-int ui__init(bool fallback_to_pager __used)
+int ui__init(void)
 {
 	int err;
 
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 761d4e9..cff18c6 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -45,27 +45,24 @@ void setup_browser(bool fallback_to_pager);
 void exit_browser(bool wait_for_ok);
 
 #ifdef NO_NEWT_SUPPORT
-static inline int ui__init(bool fallback_to_pager)
+static inline int ui__init(void)
 {
-	if (fallback_to_pager)
-		setup_pager();
-	return 0;
+	return -1;
 }
 static inline void ui__exit(bool wait_for_ok __used) {}
 #else
-int ui__init(bool fallback_to_pager);
+int ui__init(void);
 void ui__exit(bool wait_for_ok);
 #endif
 
 #ifdef NO_GTK2_SUPPORT
-static inline void perf_gtk__init(bool fallback_to_pager)
+static inline int perf_gtk__init(void)
 {
-	if (fallback_to_pager)
-		setup_pager();
+	return -1;
 }
 static inline void perf_gtk__exit(bool wait_for_ok __used) {}
 #else
-void perf_gtk__init(bool fallback_to_pager);
+int perf_gtk__init(void);
 void perf_gtk__exit(bool wait_for_ok);
 #endif
 #endif /* NO_NEWT_SUPPORT && NO_GTK2_SUPPORT */

[PATCH 6/7] perf ui: Introduce struct perf_error_ops

[Namhyung Kim]

The struct perf_error_ops is for flexible error logging.
We can register appropriate functions based on front-end.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/Makefile       |    3 +
 tools/perf/ui/gtk/util.c  |   20 ++++
 tools/perf/ui/tui/setup.c |    6 +
 tools/perf/ui/tui/util.c  |  243 +++++++++++++++++++++++++++++++++++++++
 tools/perf/ui/util.c      |  277 +++++++++------------------------------------
 tools/perf/ui/util.h      |    9 +-
 tools/perf/util/debug.c   |    2 +-
 tools/perf/util/debug.h   |   23 +++-
 8 files changed, 357 insertions(+), 226 deletions(-)
 create mode 100644 tools/perf/ui/gtk/util.c
 create mode 100644 tools/perf/ui/tui/util.c

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 5fd1d60ed8a5..d564da5c206d 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -482,6 +482,7 @@ else
 		LIB_OBJS += $(OUTPUT)ui/progress.o
 		LIB_OBJS += $(OUTPUT)ui/util.o
 		LIB_OBJS += $(OUTPUT)ui/tui/setup.o
+		LIB_OBJS += $(OUTPUT)ui/tui/util.o
 		LIB_H += ui/browser.h
 		LIB_H += ui/browsers/map.h
 		LIB_H += ui/helpline.h
@@ -505,9 +506,11 @@ else
 		EXTLIBS += $(shell pkg-config --libs gtk+-2.0)
 		LIB_OBJS += $(OUTPUT)ui/gtk/browser.o
 		LIB_OBJS += $(OUTPUT)ui/gtk/setup.o
+		LIB_OBJS += $(OUTPUT)ui/gtk/util.o
 		# Make sure that it'd be included only once.
 		ifneq ($(findstring -DNO_NEWT_SUPPORT,$(BASIC_CFLAGS)),)
 			LIB_OBJS += $(OUTPUT)ui/setup.o
+			LIB_OBJS += $(OUTPUT)ui/util.o
 		endif
 	endif
 endif
diff --git a/tools/perf/ui/gtk/util.c b/tools/perf/ui/gtk/util.c
new file mode 100644
index 000000000000..a727fe394e91
--- /dev/null
+++ b/tools/perf/ui/gtk/util.c
@@ -0,0 +1,20 @@
+#include "../util.h"
+#include "../../util/debug.h"
+#include "gtk.h"
+
+
+/*
+ * FIXME: Functions below should be implemented properly.
+ *        For now, just add stubs for NO_NEWT=1 build.
+ */
+#ifdef NO_NEWT_SUPPORT
+int ui_helpline__show_help(const char *format __used, va_list ap __used)
+{
+	return 0;
+}
+
+void ui_progress__update(u64 curr __used, u64 total __used,
+			 const char *title __used)
+{
+}
+#endif
diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index d33e943ac434..9246c09d81ad 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -15,6 +15,8 @@ pthread_mutex_t ui__lock = PTHREAD_MUTEX_INITIALIZER;
 
 static volatile int ui__need_resize;
 
+extern struct perf_error_ops perf_tui_eops;
+
 void ui__refresh_dimensions(bool force)
 {
 	if (force || ui__need_resize) {
@@ -122,6 +124,8 @@ int ui__init(void)
 	signal(SIGINT, ui__signal);
 	signal(SIGQUIT, ui__signal);
 	signal(SIGTERM, ui__signal);
+
+	perf_error__register_functions(&perf_tui_eops);
 out:
 	return err;
 }
@@ -137,4 +141,6 @@ void ui__exit(bool wait_for_ok)
 	SLsmg_refresh();
 	SLsmg_reset_smg();
 	SLang_reset_tty();
+
+	perf_error__unregister_functions(&perf_tui_eops);
 }
diff --git a/tools/perf/ui/tui/util.c b/tools/perf/ui/tui/util.c
new file mode 100644
index 000000000000..092902e30cee
--- /dev/null
+++ b/tools/perf/ui/tui/util.c
@@ -0,0 +1,243 @@
+#include "../../util/util.h"
+#include <signal.h>
+#include <stdbool.h>
+#include <string.h>
+#include <sys/ttydefaults.h>
+
+#include "../../util/cache.h"
+#include "../../util/debug.h"
+#include "../browser.h"
+#include "../keysyms.h"
+#include "../helpline.h"
+#include "../ui.h"
+#include "../util.h"
+#include "../libslang.h"
+
+static void ui_browser__argv_write(struct ui_browser *browser,
+				   void *entry, int row)
+{
+	char **arg = entry;
+	bool current_entry = ui_browser__is_current_entry(browser, row);
+
+	ui_browser__set_color(browser, current_entry ? HE_COLORSET_SELECTED :
+						       HE_COLORSET_NORMAL);
+	slsmg_write_nstring(*arg, browser->width);
+}
+
+static int popup_menu__run(struct ui_browser *menu)
+{
+	int key;
+
+	if (ui_browser__show(menu, " ", "ESC: exit, ENTER|->: Select option") < 0)
+		return -1;
+
+	while (1) {
+		key = ui_browser__run(menu, 0);
+
+		switch (key) {
+		case K_RIGHT:
+		case K_ENTER:
+			key = menu->index;
+			break;
+		case K_LEFT:
+		case K_ESC:
+		case 'q':
+		case CTRL('c'):
+			key = -1;
+			break;
+		default:
+			continue;
+		}
+
+		break;
+	}
+
+	ui_browser__hide(menu);
+	return key;
+}
+
+int ui__popup_menu(int argc, char * const argv[])
+{
+	struct ui_browser menu = {
+		.entries    = (void *)argv,
+		.refresh    = ui_browser__argv_refresh,
+		.seek	    = ui_browser__argv_seek,
+		.write	    = ui_browser__argv_write,
+		.nr_entries = argc,
+	};
+
+	return popup_menu__run(&menu);
+}
+
+int ui_browser__input_window(const char *title, const char *text, char *input,
+			     const char *exit_msg, int delay_secs)
+{
+	int x, y, len, key;
+	int max_len = 60, nr_lines = 0;
+	static char buf[50];
+	const char *t;
+
+	t = text;
+	while (1) {
+		const char *sep = strchr(t, '\n');
+
+		if (sep == NULL)
+			sep = strchr(t, '\0');
+		len = sep - t;
+		if (max_len < len)
+			max_len = len;
+		++nr_lines;
+		if (*sep == '\0')
+			break;
+		t = sep + 1;
+	}
+
+	max_len += 2;
+	nr_lines += 8;
+	y = SLtt_Screen_Rows / 2 - nr_lines / 2;
+	x = SLtt_Screen_Cols / 2 - max_len / 2;
+
+	SLsmg_set_color(0);
+	SLsmg_draw_box(y, x++, nr_lines, max_len);
+	if (title) {
+		SLsmg_gotorc(y, x + 1);
+		SLsmg_write_string((char *)title);
+	}
+	SLsmg_gotorc(++y, x);
+	nr_lines -= 7;
+	max_len -= 2;
+	SLsmg_write_wrapped_string((unsigned char *)text, y, x,
+				   nr_lines, max_len, 1);
+	y += nr_lines;
+	len = 5;
+	while (len--) {
+		SLsmg_gotorc(y + len - 1, x);
+		SLsmg_write_nstring((char *)" ", max_len);
+	}
+	SLsmg_draw_box(y++, x + 1, 3, max_len - 2);
+
+	SLsmg_gotorc(y + 3, x);
+	SLsmg_write_nstring((char *)exit_msg, max_len);
+	SLsmg_refresh();
+
+	x += 2;
+	len = 0;
+	key = ui__getch(delay_secs);
+	while (key != K_TIMER && key != K_ENTER && key != K_ESC) {
+		if (key == K_BKSPC) {
+			if (len == 0)
+				goto next_key;
+			SLsmg_gotorc(y, x + --len);
+			SLsmg_write_char(' ');
+		} else {
+			buf[len] = key;
+			SLsmg_gotorc(y, x + len++);
+			SLsmg_write_char(key);
+		}
+		SLsmg_refresh();
+
+		/* XXX more graceful overflow handling needed */
+		if (len == sizeof(buf) - 1) {
+			ui_helpline__push("maximum size of symbol name reached!");
+			key = K_ENTER;
+			break;
+		}
+next_key:
+		key = ui__getch(delay_secs);
+	}
+
+	buf[len] = '\0';
+	strncpy(input, buf, len+1);
+	return key;
+}
+
+int ui__question_window(const char *title, const char *text,
+			const char *exit_msg, int delay_secs)
+{
+	int x, y;
+	int max_len = 0, nr_lines = 0;
+	const char *t;
+
+	t = text;
+	while (1) {
+		const char *sep = strchr(t, '\n');
+		int len;
+
+		if (sep == NULL)
+			sep = strchr(t, '\0');
+		len = sep - t;
+		if (max_len < len)
+			max_len = len;
+		++nr_lines;
+		if (*sep == '\0')
+			break;
+		t = sep + 1;
+	}
+
+	max_len += 2;
+	nr_lines += 4;
+	y = SLtt_Screen_Rows / 2 - nr_lines / 2,
+	x = SLtt_Screen_Cols / 2 - max_len / 2;
+
+	SLsmg_set_color(0);
+	SLsmg_draw_box(y, x++, nr_lines, max_len);
+	if (title) {
+		SLsmg_gotorc(y, x + 1);
+		SLsmg_write_string((char *)title);
+	}
+	SLsmg_gotorc(++y, x);
+	nr_lines -= 2;
+	max_len -= 2;
+	SLsmg_write_wrapped_string((unsigned char *)text, y, x,
+				   nr_lines, max_len, 1);
+	SLsmg_gotorc(y + nr_lines - 2, x);
+	SLsmg_write_nstring((char *)" ", max_len);
+	SLsmg_gotorc(y + nr_lines - 1, x);
+	SLsmg_write_nstring((char *)exit_msg, max_len);
+	SLsmg_refresh();
+	return ui__getch(delay_secs);
+}
+
+int ui__help_window(const char *text)
+{
+	return ui__question_window("Help", text, "Press any key...", 0);
+}
+
+int ui__dialog_yesno(const char *msg)
+{
+	return ui__question_window(NULL, msg, "Enter: Yes, ESC: No", 0);
+}
+
+static int __ui__warning(const char *title, const char *format, va_list args)
+{
+	char *s;
+
+	if (vasprintf(&s, format, args) > 0) {
+		int key;
+
+		pthread_mutex_lock(&ui__lock);
+		key = ui__question_window(title, s, "Press any key...", 0);
+		pthread_mutex_unlock(&ui__lock);
+		free(s);
+		return key;
+	}
+
+	fprintf(stderr, "%s\n", title);
+	vfprintf(stderr, format, args);
+	return K_ESC;
+}
+
+static int perf_tui__error(const char *format, va_list args)
+{
+	return __ui__warning("Error:", format, args);
+}
+
+static int perf_tui__warning(const char *format, va_list args)
+{
+	return __ui__warning("Warning:", format, args);
+}
+
+struct perf_error_ops perf_tui_eops = {
+	.error		= perf_tui__error,
+	.warning	= perf_tui__warning,
+};
diff --git a/tools/perf/ui/util.c b/tools/perf/ui/util.c
index ad4374a16bb0..aa9c8345de59 100644
--- a/tools/perf/ui/util.c
+++ b/tools/perf/ui/util.c
@@ -1,250 +1,85 @@
-#include "../util.h"
-#include <signal.h>
-#include <stdbool.h>
-#include <string.h>
-#include <sys/ttydefaults.h>
-
-#include "../cache.h"
-#include "../debug.h"
-#include "browser.h"
-#include "keysyms.h"
-#include "helpline.h"
-#include "ui.h"
 #include "util.h"
-#include "libslang.h"
-
-static void ui_browser__argv_write(struct ui_browser *browser,
-				   void *entry, int row)
-{
-	char **arg = entry;
-	bool current_entry = ui_browser__is_current_entry(browser, row);
-
-	ui_browser__set_color(browser, current_entry ? HE_COLORSET_SELECTED :
-						       HE_COLORSET_NORMAL);
-	slsmg_write_nstring(*arg, browser->width);
-}
-
-static int popup_menu__run(struct ui_browser *menu)
-{
-	int key;
-
-	if (ui_browser__show(menu, " ", "ESC: exit, ENTER|->: Select option") < 0)
-		return -1;
+#include "../debug.h"
 
-	while (1) {
-		key = ui_browser__run(menu, 0);
-
-		switch (key) {
-		case K_RIGHT:
-		case K_ENTER:
-			key = menu->index;
-			break;
-		case K_LEFT:
-		case K_ESC:
-		case 'q':
-		case CTRL('c'):
-			key = -1;
-			break;
-		default:
-			continue;
-		}
-
-		break;
-	}
-
-	ui_browser__hide(menu);
-	return key;
-}
 
-int ui__popup_menu(int argc, char * const argv[])
+/*
+ * Default error logging functions
+ */
+static int perf_stdio__error(const char *format, va_list args)
 {
-	struct ui_browser menu = {
-		.entries    = (void *)argv,
-		.refresh    = ui_browser__argv_refresh,
-		.seek	    = ui_browser__argv_seek,
-		.write	    = ui_browser__argv_write,
-		.nr_entries = argc,
-	};
-
-	return popup_menu__run(&menu);
+	fprintf(stderr, "Error:\n");
+	vfprintf(stderr, format, args);
+	return 0;
 }
 
-int ui_browser__input_window(const char *title, const char *text, char *input,
-			     const char *exit_msg, int delay_secs)
+static int perf_stdio__warning(const char *format, va_list args)
 {
-	int x, y, len, key;
-	int max_len = 60, nr_lines = 0;
-	static char buf[50];
-	const char *t;
-
-	t = text;
-	while (1) {
-		const char *sep = strchr(t, '\n');
-
-		if (sep == NULL)
-			sep = strchr(t, '\0');
-		len = sep - t;
-		if (max_len < len)
-			max_len = len;
-		++nr_lines;
-		if (*sep == '\0')
-			break;
-		t = sep + 1;
-	}
-
-	max_len += 2;
-	nr_lines += 8;
-	y = SLtt_Screen_Rows / 2 - nr_lines / 2;
-	x = SLtt_Screen_Cols / 2 - max_len / 2;
-
-	SLsmg_set_color(0);
-	SLsmg_draw_box(y, x++, nr_lines, max_len);
-	if (title) {
-		SLsmg_gotorc(y, x + 1);
-		SLsmg_write_string((char *)title);
-	}
-	SLsmg_gotorc(++y, x);
-	nr_lines -= 7;
-	max_len -= 2;
-	SLsmg_write_wrapped_string((unsigned char *)text, y, x,
-				   nr_lines, max_len, 1);
-	y += nr_lines;
-	len = 5;
-	while (len--) {
-		SLsmg_gotorc(y + len - 1, x);
-		SLsmg_write_nstring((char *)" ", max_len);
-	}
-	SLsmg_draw_box(y++, x + 1, 3, max_len - 2);
-
-	SLsmg_gotorc(y + 3, x);
-	SLsmg_write_nstring((char *)exit_msg, max_len);
-	SLsmg_refresh();
-
-	x += 2;
-	len = 0;
-	key = ui__getch(delay_secs);
-	while (key != K_TIMER && key != K_ENTER && key != K_ESC) {
-		if (key == K_BKSPC) {
-			if (len == 0)
-				goto next_key;
-			SLsmg_gotorc(y, x + --len);
-			SLsmg_write_char(' ');
-		} else {
-			buf[len] = key;
-			SLsmg_gotorc(y, x + len++);
-			SLsmg_write_char(key);
-		}
-		SLsmg_refresh();
-
-		/* XXX more graceful overflow handling needed */
-		if (len == sizeof(buf) - 1) {
-			ui_helpline__push("maximum size of symbol name reached!");
-			key = K_ENTER;
-			break;
-		}
-next_key:
-		key = ui__getch(delay_secs);
-	}
-
-	buf[len] = '\0';
-	strncpy(input, buf, len+1);
-	return key;
+	fprintf(stderr, "Warning:\n");
+	vfprintf(stderr, format, args);
+	return 0;
 }
 
-int ui__question_window(const char *title, const char *text,
-			const char *exit_msg, int delay_secs)
+static struct perf_error_ops default_eops =
 {
-	int x, y;
-	int max_len = 0, nr_lines = 0;
-	const char *t;
-
-	t = text;
-	while (1) {
-		const char *sep = strchr(t, '\n');
-		int len;
-
-		if (sep == NULL)
-			sep = strchr(t, '\0');
-		len = sep - t;
-		if (max_len < len)
-			max_len = len;
-		++nr_lines;
-		if (*sep == '\0')
-			break;
-		t = sep + 1;
-	}
-
-	max_len += 2;
-	nr_lines += 4;
-	y = SLtt_Screen_Rows / 2 - nr_lines / 2,
-	x = SLtt_Screen_Cols / 2 - max_len / 2;
-
-	SLsmg_set_color(0);
-	SLsmg_draw_box(y, x++, nr_lines, max_len);
-	if (title) {
-		SLsmg_gotorc(y, x + 1);
-		SLsmg_write_string((char *)title);
-	}
-	SLsmg_gotorc(++y, x);
-	nr_lines -= 2;
-	max_len -= 2;
-	SLsmg_write_wrapped_string((unsigned char *)text, y, x,
-				   nr_lines, max_len, 1);
-	SLsmg_gotorc(y + nr_lines - 2, x);
-	SLsmg_write_nstring((char *)" ", max_len);
-	SLsmg_gotorc(y + nr_lines - 1, x);
-	SLsmg_write_nstring((char *)exit_msg, max_len);
-	SLsmg_refresh();
-	return ui__getch(delay_secs);
-}
+	.error		= perf_stdio__error,
+	.warning	= perf_stdio__warning,
+};
 
-int ui__help_window(const char *text)
-{
-	return ui__question_window("Help", text, "Press any key...", 0);
-}
+static struct perf_error_ops *perf_eops = &default_eops;
 
-int ui__dialog_yesno(const char *msg)
-{
-	return ui__question_window(NULL, msg, "Enter: Yes, ESC: No", 0);
-}
 
-int __ui__warning(const char *title, const char *format, va_list args)
+int ui__error(const char *format, ...)
 {
-	char *s;
-
-	if (use_browser > 0 && vasprintf(&s, format, args) > 0) {
-		int key;
+	int ret;
+	va_list args;
 
-		pthread_mutex_lock(&ui__lock);
-		key = ui__question_window(title, s, "Press any key...", 0);
-		pthread_mutex_unlock(&ui__lock);
-		free(s);
-		return key;
-	}
+	va_start(args, format);
+	ret = perf_eops->error(format, args);
+	va_end(args);
 
-	fprintf(stderr, "%s:\n", title);
-	vfprintf(stderr, format, args);
-	return K_ESC;
+	return ret;
 }
 
 int ui__warning(const char *format, ...)
 {
-	int key;
+	int ret;
 	va_list args;
 
 	va_start(args, format);
-	key = __ui__warning("Warning", format, args);
+	ret = perf_eops->warning(format, args);
 	va_end(args);
-	return key;
+
+	return ret;
 }
 
-int ui__error(const char *format, ...)
+
+/**
+ * perf_error__register_functions - Register error logging functions
+ * @eops: The pointer to error logging function struct
+ *
+ * Register UI-specific error logging functions. Before calling this,
+ * other logging functions should be unregistered, if any.
+ */
+int perf_error__register_functions(struct perf_error_ops *eops)
 {
-	int key;
-	va_list args;
+	if (perf_eops != &default_eops)
+		return -1;
 
-	va_start(args, format);
-	key = __ui__warning("Error", format, args);
-	va_end(args);
-	return key;
+	perf_eops = eops;
+	return 0;
+}
+
+/**
+ * perf_error__unregister_functions - Unregister error logging functions
+ * @eops: The pointer to error logging function struct
+ *
+ * Unregister already registered error logging functions.
+ */
+int perf_error__unregister_functions(struct perf_error_ops *eops)
+{
+	if (perf_eops != eops)
+		return -1;
+
+	perf_eops = &default_eops;
+	return 0;
 }
diff --git a/tools/perf/ui/util.h b/tools/perf/ui/util.h
index 2d1738bd71c8..1e0363dfffc4 100644
--- a/tools/perf/ui/util.h
+++ b/tools/perf/ui/util.h
@@ -9,6 +9,13 @@ int ui__help_window(const char *text);
 int ui__dialog_yesno(const char *msg);
 int ui__question_window(const char *title, const char *text,
 			const char *exit_msg, int delay_secs);
-int __ui__warning(const char *title, const char *format, va_list args);
+
+struct perf_error_ops {
+	int (*error)(const char *format, va_list args);
+	int (*warning)(const char *format, va_list args);
+};
+
+int perf_error__register_functions(struct perf_error_ops *eops);
+int perf_error__unregister_functions(struct perf_error_ops *eops);
 
 #endif /* _PERF_UI_UTIL_H_ */
diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
index 26817daa2961..8693f018731d 100644
--- a/tools/perf/util/debug.c
+++ b/tools/perf/util/debug.c
@@ -46,7 +46,7 @@ int dump_printf(const char *fmt, ...)
 	return ret;
 }
 
-#ifdef NO_NEWT_SUPPORT
+#if defined(NO_NEWT_SUPPORT) && defined(NO_GTK2_SUPPORT)
 int ui__warning(const char *format, ...)
 {
 	va_list args;
diff --git a/tools/perf/util/debug.h b/tools/perf/util/debug.h
index 6bebe7f0a20c..f9674a138ddf 100644
--- a/tools/perf/util/debug.h
+++ b/tools/perf/util/debug.h
@@ -12,8 +12,9 @@ int dump_printf(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
 void trace_event(union perf_event *event);
 
 struct ui_progress;
+struct perf_error_ops;
 
-#ifdef NO_NEWT_SUPPORT
+#if defined(NO_NEWT_SUPPORT) && defined(NO_GTK2_SUPPORT)
 static inline int ui_helpline__show_help(const char *format __used, va_list ap __used)
 {
 	return 0;
@@ -23,12 +24,28 @@ static inline void ui_progress__update(u64 curr __used, u64 total __used,
 				       const char *title __used) {}
 
 #define ui__error(format, arg...) ui__warning(format, ##arg)
-#else
+
+static inline int
+perf_error__register_functions(struct perf_error_ops *eops __used)
+{
+	return 0;
+}
+
+static inline int
+perf_error__unregister_functions(struct perf_error_ops *eops __used)
+{
+	return 0;
+}
+
+#else /* NO_NEWT_SUPPORT && NO_GTK2_SUPPORT */
+
 extern char ui_helpline__last_msg[];
 int ui_helpline__show_help(const char *format, va_list ap);
 #include "../ui/progress.h"
 int ui__error(const char *format, ...) __attribute__((format(printf, 1, 2)));
-#endif
+#include "../ui/util.h"
+
+#endif /* NO_NEWT_SUPPORT && NO_GTK2_SUPPORT */
 
 int ui__warning(const char *format, ...) __attribute__((format(printf, 1, 2)));
 int ui__error_paranoid(void);
-- 
1.7.10

Re: [PATCH 6/7] perf ui: Introduce struct perf_error_ops

[Arnaldo Carvalho de Melo]

Em Mon, Apr 30, 2012 at 01:55:10PM +0900, Namhyung Kim escreveu:
> The struct perf_error_ops is for flexible error logging.
> We can register appropriate functions based on front-end.
> 
> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> +
> +	perf_error__register_functions(&perf_tui_eops);

just perf_error__register() is enough, I think.


- Arnaldo

Re: [PATCH 6/7] perf ui: Introduce struct perf_error_ops

[Namhyung Kim]

2012-05-02 (수), 16:03 -0300, Arnaldo Carvalho de Melo:
> Em Mon, Apr 30, 2012 at 01:55:10PM +0900, Namhyung Kim escreveu:
> > The struct perf_error_ops is for flexible error logging.
> > We can register appropriate functions based on front-end.
> > 
> > Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> > +
> > +	perf_error__register_functions(&perf_tui_eops);
> 
> just perf_error__register() is enough, I think.
> 

Okay, will fix.

Thanks,
Namhyung

[PATCH 7/7] perf ui/gtk: Use struct perf_error_ops

[Namhyung Kim]

Define and use perf_gtk_eops to provide a GTK2 message
dialog for error reporting. To do that, we need global
main_window variable for tracking UI state.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/ui/gtk/browser.c |    6 ++++--
 tools/perf/ui/gtk/gtk.h     |    4 ++++
 tools/perf/ui/gtk/setup.c   |    6 ++++++
 tools/perf/ui/gtk/util.c    |   47 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index 0656c381a89c..64286437a553 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -12,7 +12,7 @@
 static void perf_gtk__signal(int sig)
 {
 	psignal(sig, "perf");
-	gtk_main_quit();
+	perf_gtk__exit(false);
 }
 
 static void perf_gtk__resize_window(GtkWidget *window)
@@ -137,7 +137,7 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 	signal(SIGQUIT, perf_gtk__signal);
 	signal(SIGTERM, perf_gtk__signal);
 
-	window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
+	main_window = window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
 
 	gtk_window_set_title(GTK_WINDOW(window), "perf report");
 
@@ -174,5 +174,7 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 
 	gtk_main();
 
+	main_window = NULL;
+
 	return 0;
 }
diff --git a/tools/perf/ui/gtk/gtk.h b/tools/perf/ui/gtk/gtk.h
index 75177ee04032..eb08cbea0da1 100644
--- a/tools/perf/ui/gtk/gtk.h
+++ b/tools/perf/ui/gtk/gtk.h
@@ -5,4 +5,8 @@
 #include <gtk/gtk.h>
 #pragma GCC diagnostic error "-Wstrict-prototypes"
 
+#include "../../util/debug.h"
+
+extern GtkWidget *main_window;
+
 #endif /* _PERF_GTK_H_ */
diff --git a/tools/perf/ui/gtk/setup.c b/tools/perf/ui/gtk/setup.c
index 829529957766..ef74e043eba2 100644
--- a/tools/perf/ui/gtk/setup.c
+++ b/tools/perf/ui/gtk/setup.c
@@ -1,12 +1,18 @@
 #include "gtk.h"
 #include "../../util/cache.h"
+#include "../../util/debug.h"
+
+
+extern struct perf_error_ops perf_gtk_eops;
 
 int perf_gtk__init(void)
 {
+	perf_error__register_functions(&perf_gtk_eops);
 	return gtk_init_check(NULL, NULL) ? 0 : -1;
 }
 
 void perf_gtk__exit(bool wait_for_ok __used)
 {
+	perf_error__unregister_functions(&perf_gtk_eops);
 	gtk_main_quit();
 }
diff --git a/tools/perf/ui/gtk/util.c b/tools/perf/ui/gtk/util.c
index a727fe394e91..0d0ed2ed3937 100644
--- a/tools/perf/ui/gtk/util.c
+++ b/tools/perf/ui/gtk/util.c
@@ -2,6 +2,53 @@
 #include "../../util/debug.h"
 #include "gtk.h"
 
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+
+GtkWidget *main_window;
+
+static int message_dialog(const char *title, const char *format, va_list args)
+{
+	char *msg;
+	GtkWidget *dialog;
+	GtkMessageType message_type = GTK_MESSAGE_WARNING;
+
+	if (!main_window || vasprintf(&msg, format, args) < 0) {
+		fprintf(stderr, "%s:\n", title);
+		vfprintf(stderr, format, args);
+		return -1;
+	}
+
+	if (strcmp(title, "Error") == 0)
+		message_type = GTK_MESSAGE_ERROR;
+
+	dialog = gtk_message_dialog_new_with_markup(GTK_WINDOW(main_window),
+					GTK_DIALOG_DESTROY_WITH_PARENT,
+					message_type,
+					GTK_BUTTONS_CLOSE,
+					"<b>%s</b>\n\n%s", title, msg);
+	gtk_dialog_run(GTK_DIALOG(dialog));
+	gtk_widget_destroy(dialog);
+	free(msg);
+	return 0;
+}
+
+static int perf_gtk__error(const char *format, va_list args)
+{
+	return message_dialog("Error", format, args);
+}
+
+static int perf_gtk__warning(const char *format, va_list args)
+{
+	return message_dialog("Warning", format, args);
+}
+
+struct perf_error_ops perf_gtk_eops = {
+	.error		= perf_gtk__error,
+	.warning	= perf_gtk__warning,
+};
 
 /*
  * FIXME: Functions below should be implemented properly.
-- 
1.7.10

Re: [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops

[Pekka Enberg]

On Mon, 30 Apr 2012, Namhyung Kim wrote:
> Define and use perf_gtk_eops to provide a GTK2 message
> dialog for error reporting. To do that, we need global
> main_window variable for tracking UI state.
> 
> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> diff --git a/tools/perf/ui/gtk/util.c b/tools/perf/ui/gtk/util.c
> index a727fe394e91..0d0ed2ed3937 100644
> --- a/tools/perf/ui/gtk/util.c
> +++ b/tools/perf/ui/gtk/util.c
> @@ -2,6 +2,53 @@
>  #include "../../util/debug.h"
>  #include "gtk.h"
>  
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +
> +
> +GtkWidget *main_window;
> +
> +static int message_dialog(const char *title, const char *format, va_list args)
> +{
> +	char *msg;
> +	GtkWidget *dialog;
> +	GtkMessageType message_type = GTK_MESSAGE_WARNING;
> +
> +	if (!main_window || vasprintf(&msg, format, args) < 0) {
> +		fprintf(stderr, "%s:\n", title);
> +		vfprintf(stderr, format, args);
> +		return -1;
> +	}
> +
> +	if (strcmp(title, "Error") == 0)
> +		message_type = GTK_MESSAGE_ERROR;
> +
> +	dialog = gtk_message_dialog_new_with_markup(GTK_WINDOW(main_window),
> +					GTK_DIALOG_DESTROY_WITH_PARENT,
> +					message_type,
> +					GTK_BUTTONS_CLOSE,
> +					"<b>%s</b>\n\n%s", title, msg);
> +	gtk_dialog_run(GTK_DIALOG(dialog));
> +	gtk_widget_destroy(dialog);
> +	free(msg);
> +	return 0;
> +}

I think this is an usability glitch waiting to happen - especially so if 
you use it for warnings. There's no reason to require the user to react to 
warning messages in the GUI because there's absolutely nothing they can do 
about them.

I guess we could do something like the "ui helpline" thing used by the 
newt front-end if we really wanted to.

			Pekka

Re: [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops

[Namhyung Kim]

Hi,

On Mon, 30 Apr 2012 09:31:28 +0300 (EEST), Pekka Enberg wrote:
> On Mon, 30 Apr 2012, Namhyung Kim wrote:
>> Define and use perf_gtk_eops to provide a GTK2 message
>> dialog for error reporting. To do that, we need global
>> main_window variable for tracking UI state.
>> 
>> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
>> diff --git a/tools/perf/ui/gtk/util.c b/tools/perf/ui/gtk/util.c
>> index a727fe394e91..0d0ed2ed3937 100644
>> --- a/tools/perf/ui/gtk/util.c
>> +++ b/tools/perf/ui/gtk/util.c
>> @@ -2,6 +2,53 @@
>>  #include "../../util/debug.h"
>>  #include "gtk.h"
>>  
>> +#include <stdio.h>
>> +#include <string.h>
>> +#include <stdlib.h>
>> +
>> +
>> +GtkWidget *main_window;
>> +
>> +static int message_dialog(const char *title, const char *format, va_list args)
>> +{
>> +	char *msg;
>> +	GtkWidget *dialog;
>> +	GtkMessageType message_type = GTK_MESSAGE_WARNING;
>> +
>> +	if (!main_window || vasprintf(&msg, format, args) < 0) {
>> +		fprintf(stderr, "%s:\n", title);
>> +		vfprintf(stderr, format, args);
>> +		return -1;
>> +	}
>> +
>> +	if (strcmp(title, "Error") == 0)
>> +		message_type = GTK_MESSAGE_ERROR;
>> +
>> +	dialog = gtk_message_dialog_new_with_markup(GTK_WINDOW(main_window),
>> +					GTK_DIALOG_DESTROY_WITH_PARENT,
>> +					message_type,
>> +					GTK_BUTTONS_CLOSE,
>> +					"<b>%s</b>\n\n%s", title, msg);
>> +	gtk_dialog_run(GTK_DIALOG(dialog));
>> +	gtk_widget_destroy(dialog);
>> +	free(msg);
>> +	return 0;
>> +}
>
> I think this is an usability glitch waiting to happen - especially so if 
> you use it for warnings. There's no reason to require the user to react to 
> warning messages in the GUI because there's absolutely nothing they can do 
> about them.
>
> I guess we could do something like the "ui helpline" thing used by the 
> newt front-end if we really wanted to.
>
> 			Pekka

I did quick grep on ui__warning and found that most of its users are
trying to show the messages before exiting. I think some (at least) of
them can be converted to ui__error(). And as existing implementation
(TUI) already requires user input for this, I thought it's ok.

But I agreed with you that ui__warning should not be used for showing
non-critical messages and converted to helpline-style ones.

Thanks,
Namhyung

Re: [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops

[Ingo Molnar]

* Namhyung Kim <namhyung.kim@lge.com> wrote:

> Hi,
> 
> On Mon, 30 Apr 2012 09:31:28 +0300 (EEST), Pekka Enberg wrote:
> > On Mon, 30 Apr 2012, Namhyung Kim wrote:
> >> Define and use perf_gtk_eops to provide a GTK2 message
> >> dialog for error reporting. To do that, we need global
> >> main_window variable for tracking UI state.
> >> 
> >> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> >> diff --git a/tools/perf/ui/gtk/util.c b/tools/perf/ui/gtk/util.c
> >> index a727fe394e91..0d0ed2ed3937 100644
> >> --- a/tools/perf/ui/gtk/util.c
> >> +++ b/tools/perf/ui/gtk/util.c
> >> @@ -2,6 +2,53 @@
> >>  #include "../../util/debug.h"
> >>  #include "gtk.h"
> >>  
> >> +#include <stdio.h>
> >> +#include <string.h>
> >> +#include <stdlib.h>
> >> +
> >> +
> >> +GtkWidget *main_window;
> >> +
> >> +static int message_dialog(const char *title, const char *format, va_list args)
> >> +{
> >> +	char *msg;
> >> +	GtkWidget *dialog;
> >> +	GtkMessageType message_type = GTK_MESSAGE_WARNING;
> >> +
> >> +	if (!main_window || vasprintf(&msg, format, args) < 0) {
> >> +		fprintf(stderr, "%s:\n", title);
> >> +		vfprintf(stderr, format, args);
> >> +		return -1;
> >> +	}
> >> +
> >> +	if (strcmp(title, "Error") == 0)
> >> +		message_type = GTK_MESSAGE_ERROR;
> >> +
> >> +	dialog = gtk_message_dialog_new_with_markup(GTK_WINDOW(main_window),
> >> +					GTK_DIALOG_DESTROY_WITH_PARENT,
> >> +					message_type,
> >> +					GTK_BUTTONS_CLOSE,
> >> +					"<b>%s</b>\n\n%s", title, msg);
> >> +	gtk_dialog_run(GTK_DIALOG(dialog));
> >> +	gtk_widget_destroy(dialog);
> >> +	free(msg);
> >> +	return 0;
> >> +}
> >
> > I think this is an usability glitch waiting to happen - especially so if 
> > you use it for warnings. There's no reason to require the user to react to 
> > warning messages in the GUI because there's absolutely nothing they can do 
> > about them.
> >
> > I guess we could do something like the "ui helpline" thing used by the 
> > newt front-end if we really wanted to.
> >
> > 			Pekka
> 
> I did quick grep on ui__warning and found that most of its 
> users are trying to show the messages before exiting. I think 
> some (at least) of them can be converted to ui__error(). And 
> as existing implementation (TUI) already requires user input 
> for this, I thought it's ok.
> 
> But I agreed with you that ui__warning should not be used for 
> showing non-critical messages and converted to helpline-style 
> ones.

If they are in essence ui__error() already then please convert 
them to ui__error() instead of perpetuating the mistake - don't 
force annoying pop-ups for warnings that may or may not be 
fatal. Spurious pop-ups are sad and people hate them.

Thanks,

	Ingo

Re: [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops

[Namhyung Kim]

Hi,

On Mon, 7 May 2012 10:10:51 +0200, Ingo Molnar wrote:
> * Namhyung Kim <namhyung.kim@lge.com> wrote:
>
>> Hi,
>> 
>> On Mon, 30 Apr 2012 09:31:28 +0300 (EEST), Pekka Enberg wrote:
>> > On Mon, 30 Apr 2012, Namhyung Kim wrote:
>> >> Define and use perf_gtk_eops to provide a GTK2 message
>> >> dialog for error reporting. To do that, we need global
>> >> main_window variable for tracking UI state.
>> >> 
>> > I think this is an usability glitch waiting to happen - especially so if 
>> > you use it for warnings. There's no reason to require the user to react to 
>> > warning messages in the GUI because there's absolutely nothing they can do 
>> > about them.
>> >
>> > I guess we could do something like the "ui helpline" thing used by the 
>> > newt front-end if we really wanted to.
>> >
>> > 			Pekka
>> 
>> I did quick grep on ui__warning and found that most of its 
>> users are trying to show the messages before exiting. I think 
>> some (at least) of them can be converted to ui__error(). And 
>> as existing implementation (TUI) already requires user input 
>> for this, I thought it's ok.
>> 
>> But I agreed with you that ui__warning should not be used for 
>> showing non-critical messages and converted to helpline-style 
>> ones.
>
> If they are in essence ui__error() already then please convert 
> them to ui__error() instead of perpetuating the mistake - don't 
> force annoying pop-ups for warnings that may or may not be 
> fatal. Spurious pop-ups are sad and people hate them.
>

Ok, will do that in another patch (series). So you mean ui__warning
should not be a pop-up dialog, right?

Thanks,
Namhyung

Re: [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops

[Ingo Molnar]

* Namhyung Kim <namhyung.kim@lge.com> wrote:

> Hi,
> 
> On Mon, 7 May 2012 10:10:51 +0200, Ingo Molnar wrote:
> > * Namhyung Kim <namhyung.kim@lge.com> wrote:
> >
> >> Hi,
> >> 
> >> On Mon, 30 Apr 2012 09:31:28 +0300 (EEST), Pekka Enberg wrote:
> >> > On Mon, 30 Apr 2012, Namhyung Kim wrote:
> >> >> Define and use perf_gtk_eops to provide a GTK2 message
> >> >> dialog for error reporting. To do that, we need global
> >> >> main_window variable for tracking UI state.
> >> >> 
> >> > I think this is an usability glitch waiting to happen - especially so if 
> >> > you use it for warnings. There's no reason to require the user to react to 
> >> > warning messages in the GUI because there's absolutely nothing they can do 
> >> > about them.
> >> >
> >> > I guess we could do something like the "ui helpline" thing used by the 
> >> > newt front-end if we really wanted to.
> >> >
> >> > 			Pekka
> >> 
> >> I did quick grep on ui__warning and found that most of its 
> >> users are trying to show the messages before exiting. I think 
> >> some (at least) of them can be converted to ui__error(). And 
> >> as existing implementation (TUI) already requires user input 
> >> for this, I thought it's ok.
> >> 
> >> But I agreed with you that ui__warning should not be used for 
> >> showing non-critical messages and converted to helpline-style 
> >> ones.
> >
> > If they are in essence ui__error() already then please convert 
> > them to ui__error() instead of perpetuating the mistake - don't 
> > force annoying pop-ups for warnings that may or may not be 
> > fatal. Spurious pop-ups are sad and people hate them.
> >
> 
> Ok, will do that in another patch (series). So you mean 
> ui__warning should not be a pop-up dialog, right?

Pekka, is that what you meant too?

If there are *real* warnings that are not followed up by some 
fatal close of the application then those should probably be 
displayed in some sort of unintrusive manner - a scrolling list 
of events, a small status display box that can be clicked on if 
people get curious, etc.

Small, well-thought out details like that are nice in a GUI. 
Create enough of them and people start not to hate the app 
(people generally hate computers, so GUIs have an uphill 
struggle). Break a critical mass and people will actively love 
the app.

Thanks,

	Ingo