mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end (v3)
@ 2012-05-29  4:22 Namhyung Kim
  2012-05-29  4:22 ` [PATCH RESEND 1/7] perf ui: Make --stdio default when TUI is not supported Namhyung Kim
                   ` (7 more replies)
  0 siblings, 8 replies; 22+ messages in thread
From: Namhyung Kim @ 2012-05-29  4:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Pekka Enberg, LKML

Hi,

This patchset implements ui__warning/error() functions for GTK2.
Patch 1 is a resend (which forgot to cc LKML) and independent of
this series. Patch 2 converts invocations of ui__warning followed
by exit (or similar effect) into ui__error.

I chose GtkInfoBar widget to implement ui__warning since it provides
a simple way to do what we want IMHO. In fact, a guy sat next to me
at the Korean GNOME hackfest suggested to use it. The only problem
is it requires 2.18 (or newer) version of GTK+ libraries. So I added
status bar as a fallback when info bar is not supported, and this
can be used to implement helpline functions later.

Changes from v2:
 * Convert critical use of ui__warning to ui__error (by Ingo)
 * Use GtkInfoBar instead of a message dialog (by Pekka)

Any comments are welcome.

Thanks,
Namhyung


Namhyung Kim (7):
  perf ui: Make --stdio default when TUI is not supported
  perf tools: Convert critical messages to ui__error()
  perf ui: Introduce struct perf_error_ops
  perf ui/gtk: Introduce struct perf_gtk_context
  perf ui/gtk: Add GTK statusbar widget to browser window
  perf ui/gtk: Add GTK info_bar widget to browser window
  perf ui/gtk: Use struct perf_error_ops

 tools/perf/Makefile                 |    6 +
 tools/perf/builtin-annotate.c       |    2 +-
 tools/perf/builtin-record.c         |   14 +-
 tools/perf/builtin-report.c         |   14 +-
 tools/perf/builtin-top.c            |   22 +--
 tools/perf/config/feature-tests.mak |   13 ++
 tools/perf/ui/gtk/browser.c         |   67 ++++++++-
 tools/perf/ui/gtk/gtk.h             |   31 ++++
 tools/perf/ui/gtk/setup.c           |    5 +
 tools/perf/ui/gtk/util.c            |  129 ++++++++++++++++
 tools/perf/ui/setup.c               |    1 +
 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 ++-
 17 files changed, 609 insertions(+), 255 deletions(-)
 create mode 100644 tools/perf/ui/gtk/util.c
 create mode 100644 tools/perf/ui/tui/util.c

-- 
1.7.9.2


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH RESEND 1/7] perf ui: Make --stdio default when TUI is not supported
  2012-05-29  4:22 [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end (v3) Namhyung Kim
@ 2012-05-29  4:22 ` Namhyung Kim
  2012-05-29  4:22 ` [PATCH 2/7] perf tools: Convert critical messages to ui__error() Namhyung Kim
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: Namhyung Kim @ 2012-05-29  4:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Pekka Enberg, LKML

The commit dc41b9b8f02db ("perf ui: Change fallback policy of
setup_browser") changed default behavior of the function but
missed setting the use_browser variable to 0 accidently. So
perf report ends up doing nothing in such cases. Fix it.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 tools/perf/ui/setup.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index 9f5f888f73e3..791fb15ce350 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -22,6 +22,7 @@ void setup_browser(bool fallback_to_pager)
 			break;
 		/* fall through */
 	default:
+		use_browser = 0;
 		if (fallback_to_pager)
 			setup_pager();
 		break;
-- 
1.7.9.2


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 2/7] perf tools: Convert critical messages to ui__error()
  2012-05-29  4:22 [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end (v3) Namhyung Kim
  2012-05-29  4:22 ` [PATCH RESEND 1/7] perf ui: Make --stdio default when TUI is not supported Namhyung Kim
@ 2012-05-29  4:22 ` Namhyung Kim
  2012-05-30  7:49   ` [tip:perf/urgent] " tip-bot for Namhyung Kim
  2012-05-29  4:22 ` [PATCH 3/7] perf ui: Introduce struct perf_error_ops Namhyung Kim
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: Namhyung Kim @ 2012-05-29  4:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Pekka Enberg, LKML

There were places where use ui__warning (or even fprintf) to
show critical messages. This patch converts them to ui__error
so that the front-end code can implement appropriate behavior.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 tools/perf/builtin-annotate.c |    2 +-
 tools/perf/builtin-record.c   |   14 +++++++-------
 tools/perf/builtin-report.c   |   14 ++++++--------
 tools/perf/builtin-top.c      |   22 +++++++++++-----------
 4 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 806e0a286634..67522cf87405 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -215,7 +215,7 @@ static int __cmd_annotate(struct perf_annotate *ann)
 	}
 
 	if (total_nr_samples == 0) {
-		ui__warning("The %s file has no samples!\n", session->filename);
+		ui__error("The %s file has no samples!\n", session->filename);
 		goto out_delete;
 	}
 out_delete:
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index e5cb08427e13..f95840d04e4c 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -264,7 +264,7 @@ try_again:
 			}
 
 			if (err == ENOENT) {
-				ui__warning("The %s event is not supported.\n",
+				ui__error("The %s event is not supported.\n",
 					    event_name(pos));
 				exit(EXIT_FAILURE);
 			}
@@ -858,8 +858,8 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		usage_with_options(record_usage, record_options);
 
 	if (rec->force && rec->append_file) {
-		fprintf(stderr, "Can't overwrite and append at the same time."
-				" You need to choose between -f and -A");
+		ui__error("Can't overwrite and append at the same time."
+			  " You need to choose between -f and -A");
 		usage_with_options(record_usage, record_options);
 	} else if (rec->append_file) {
 		rec->write_mode = WRITE_APPEND;
@@ -868,8 +868,8 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 	}
 
 	if (nr_cgroups && !rec->opts.target.system_wide) {
-		fprintf(stderr, "cgroup monitoring only available in"
-			" system-wide mode\n");
+		ui__error("cgroup monitoring only available in"
+			  " system-wide mode\n");
 		usage_with_options(record_usage, record_options);
 	}
 
@@ -905,7 +905,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		int saved_errno = errno;
 
 		perf_target__strerror(&rec->opts.target, err, errbuf, BUFSIZ);
-		ui__warning("%s", errbuf);
+		ui__error("%s", errbuf);
 
 		err = -saved_errno;
 		goto out_free_fd;
@@ -933,7 +933,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 	else if (rec->opts.freq) {
 		rec->opts.default_interval = rec->opts.freq;
 	} else {
-		fprintf(stderr, "frequency and count are zero, aborting\n");
+		ui__error("frequency and count are zero, aborting\n");
 		err = -EINVAL;
 		goto out_free_fd;
 	}
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index d58e41445d0d..8c767c6bca91 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -251,13 +251,13 @@ static int perf_report__setup_sample_type(struct perf_report *rep)
 
 	if (!(self->sample_type & PERF_SAMPLE_CALLCHAIN)) {
 		if (sort__has_parent) {
-			ui__warning("Selected --sort parent, but no "
+			ui__error("Selected --sort parent, but no "
 				    "callchain data. Did you call "
 				    "'perf record' without -g?\n");
 			return -EINVAL;
 		}
 		if (symbol_conf.use_callchain) {
-			ui__warning("Selected -g but no callchain data. Did "
+			ui__error("Selected -g but no callchain data. Did "
 				    "you call 'perf record' without -g?\n");
 			return -1;
 		}
@@ -266,17 +266,15 @@ static int perf_report__setup_sample_type(struct perf_report *rep)
 		   !symbol_conf.use_callchain) {
 			symbol_conf.use_callchain = true;
 			if (callchain_register_param(&callchain_param) < 0) {
-				ui__warning("Can't register callchain "
-					    "params.\n");
+				ui__error("Can't register callchain params.\n");
 				return -EINVAL;
 			}
 	}
 
 	if (sort__branch_mode == 1) {
 		if (!(self->sample_type & PERF_SAMPLE_BRANCH_STACK)) {
-			fprintf(stderr, "selected -b but no branch data."
-					" Did you call perf record without"
-					" -b?\n");
+			ui__error("Selected -b but no branch data. "
+				  "Did you call perf record without -b?\n");
 			return -1;
 		}
 	}
@@ -420,7 +418,7 @@ static int __cmd_report(struct perf_report *rep)
 	}
 
 	if (nr_samples == 0) {
-		ui__warning("The %s file has no samples!\n", session->filename);
+		ui__error("The %s file has no samples!\n", session->filename);
 		goto out_delete;
 	}
 
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 6031dce0429f..906cccb5b4db 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -959,16 +959,16 @@ try_again:
 			}
 
 			if (err == ENOENT) {
-				ui__warning("The %s event is not supported.\n",
+				ui__error("The %s event is not supported.\n",
 					    event_name(counter));
 				goto out_err;
 			} else if (err == EMFILE) {
-				ui__warning("Too many events are opened.\n"
+				ui__error("Too many events are opened.\n"
 					    "Try again after reducing the number of events\n");
 				goto out_err;
 			}
 
-			ui__warning("The sys_perf_event_open() syscall "
+			ui__error("The sys_perf_event_open() syscall "
 				    "returned with %d (%s).  /bin/dmesg "
 				    "may provide additional information.\n"
 				    "No CONFIG_PERF_EVENTS=y kernel support "
@@ -978,7 +978,7 @@ try_again:
 	}
 
 	if (perf_evlist__mmap(evlist, top->mmap_pages, false) < 0) {
-		ui__warning("Failed to mmap with %d (%s)\n",
+		ui__error("Failed to mmap with %d (%s)\n",
 			    errno, strerror(errno));
 		goto out_err;
 	}
@@ -994,12 +994,12 @@ static int perf_top__setup_sample_type(struct perf_top *top)
 {
 	if (!top->sort_has_symbols) {
 		if (symbol_conf.use_callchain) {
-			ui__warning("Selected -g but \"sym\" not present in --sort/-s.");
+			ui__error("Selected -g but \"sym\" not present in --sort/-s.");
 			return -EINVAL;
 		}
 	} else if (!top->dont_use_callchains && callchain_param.mode != CHAIN_NONE) {
 		if (callchain_register_param(&callchain_param) < 0) {
-			ui__warning("Can't register callchain params.\n");
+			ui__error("Can't register callchain params.\n");
 			return -EINVAL;
 		}
 	}
@@ -1041,7 +1041,7 @@ static int __cmd_top(struct perf_top *top)
 
 	if (pthread_create(&thread, NULL, (use_browser > 0 ? display_thread_tui :
 							    display_thread), top)) {
-		printf("Could not create display thread.\n");
+		ui__error("Could not create display thread.\n");
 		exit(-1);
 	}
 
@@ -1050,7 +1050,7 @@ static int __cmd_top(struct perf_top *top)
 
 		param.sched_priority = top->realtime_prio;
 		if (sched_setscheduler(0, SCHED_FIFO, &param)) {
-			printf("Could not set realtime priority.\n");
+			ui__error("Could not set realtime priority.\n");
 			exit(-1);
 		}
 	}
@@ -1274,7 +1274,7 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 		int saved_errno = errno;
 
 		perf_target__strerror(&top.target, status, errbuf, BUFSIZ);
-		ui__warning("%s", errbuf);
+		ui__error("%s", errbuf);
 
 		status = -saved_errno;
 		goto out_delete_evlist;
@@ -1288,7 +1288,7 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 
 	if (!top.evlist->nr_entries &&
 	    perf_evlist__add_default(top.evlist) < 0) {
-		pr_err("Not enough memory for event selector list\n");
+		ui__error("Not enough memory for event selector list\n");
 		return -ENOMEM;
 	}
 
@@ -1305,7 +1305,7 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 	else if (top.freq) {
 		top.default_interval = top.freq;
 	} else {
-		fprintf(stderr, "frequency and count are zero, aborting\n");
+		ui__error("frequency and count are zero, aborting\n");
 		exit(EXIT_FAILURE);
 	}
 
-- 
1.7.9.2


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/7] perf ui: Introduce struct perf_error_ops
  2012-05-29  4:22 [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end (v3) Namhyung Kim
  2012-05-29  4:22 ` [PATCH RESEND 1/7] perf ui: Make --stdio default when TUI is not supported Namhyung Kim
  2012-05-29  4:22 ` [PATCH 2/7] perf tools: Convert critical messages to ui__error() Namhyung Kim
@ 2012-05-29  4:22 ` Namhyung Kim
  2012-06-20 16:45   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2012-05-29  4:22 ` [PATCH 4/7] perf ui/gtk: Introduce struct perf_gtk_context Namhyung Kim
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: Namhyung Kim @ 2012-05-29  4:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Pekka Enberg, LKML,
	Namhyung Kim

From: Namhyung Kim <namhyung.kim@lge.com>

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 1d3d513beb9b..7034b3c70d42 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -503,6 +503,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
@@ -526,9 +527,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..e813c1d17346 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(&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(&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..4f989774c8c6 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 - 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(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 - Unregister error logging functions
+ * @eops: The pointer to error logging function struct
+ *
+ * Unregister already registered error logging functions.
+ */
+int perf_error__unregister(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..361f08c52d37 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(struct perf_error_ops *eops);
+int perf_error__unregister(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 efb1fce259a4..4dfe0bb3c322 100644
--- a/tools/perf/util/debug.c
+++ b/tools/perf/util/debug.c
@@ -47,7 +47,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..015c91dbc096 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(struct perf_error_ops *eops __used)
+{
+	return 0;
+}
+
+static inline int
+perf_error__unregister(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.9.2


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 4/7] perf ui/gtk: Introduce struct perf_gtk_context
  2012-05-29  4:22 [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end (v3) Namhyung Kim
                   ` (2 preceding siblings ...)
  2012-05-29  4:22 ` [PATCH 3/7] perf ui: Introduce struct perf_error_ops Namhyung Kim
@ 2012-05-29  4:22 ` Namhyung Kim
  2012-06-20 16:46   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2012-05-29  4:23 ` [PATCH 5/7] perf ui/gtk: Add GTK statusbar widget to browser window Namhyung Kim
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: Namhyung Kim @ 2012-05-29  4:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Pekka Enberg, LKML

The struct perf_gtk_context is for tracking current state of
GTK window and/or other things. This is a preparation of next
changes.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 tools/perf/ui/gtk/browser.c |    8 +++++++-
 tools/perf/ui/gtk/gtk.h     |   17 +++++++++++++++++
 tools/perf/ui/gtk/util.c    |   23 +++++++++++++++++++++++
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index 0656c381a89c..33ab1aee3472 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -11,8 +11,8 @@
 
 static void perf_gtk__signal(int sig)
 {
+	perf_gtk__exit(false);
 	psignal(sig, "perf");
-	gtk_main_quit();
 }
 
 static void perf_gtk__resize_window(GtkWidget *window)
@@ -143,6 +143,10 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 
 	g_signal_connect(window, "delete_event", gtk_main_quit, NULL);
 
+	pgctx = perf_gtk__activate_context(window);
+	if (!pgctx)
+		return -1;
+
 	notebook = gtk_notebook_new();
 
 	list_for_each_entry(pos, &evlist->entries, node) {
@@ -174,5 +178,7 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 
 	gtk_main();
 
+	perf_gtk__deactivate_context(&pgctx);
+
 	return 0;
 }
diff --git a/tools/perf/ui/gtk/gtk.h b/tools/perf/ui/gtk/gtk.h
index 75177ee04032..34fbca6d48a2 100644
--- a/tools/perf/ui/gtk/gtk.h
+++ b/tools/perf/ui/gtk/gtk.h
@@ -1,8 +1,25 @@
 #ifndef _PERF_GTK_H_
 #define _PERF_GTK_H_ 1
 
+#include <stdbool.h>
+
 #pragma GCC diagnostic ignored "-Wstrict-prototypes"
 #include <gtk/gtk.h>
 #pragma GCC diagnostic error "-Wstrict-prototypes"
 
+
+struct perf_gtk_context {
+	GtkWidget *main_window;
+};
+
+extern struct perf_gtk_context *pgctx;
+
+static inline bool perf_gtk__is_active_context(struct perf_gtk_context *ctx)
+{
+	return ctx && ctx->main_window;
+}
+
+struct perf_gtk_context *perf_gtk__activate_context(GtkWidget *window);
+int perf_gtk__deactivate_context(struct perf_gtk_context **ctx);
+
 #endif /* _PERF_GTK_H_ */
diff --git a/tools/perf/ui/gtk/util.c b/tools/perf/ui/gtk/util.c
index a727fe394e91..6fe13fdc513e 100644
--- a/tools/perf/ui/gtk/util.c
+++ b/tools/perf/ui/gtk/util.c
@@ -3,6 +3,29 @@
 #include "gtk.h"
 
 
+struct perf_gtk_context *pgctx;
+
+struct perf_gtk_context *perf_gtk__activate_context(GtkWidget *window)
+{
+	struct perf_gtk_context *ctx;
+
+	ctx = malloc(sizeof(*pgctx));
+	if (ctx)
+		ctx->main_window = window;
+
+	return ctx;
+}
+
+int perf_gtk__deactivate_context(struct perf_gtk_context **ctx)
+{
+	if (!perf_gtk__is_active_context(*ctx))
+		return -1;
+
+	free(*ctx);
+	*ctx = NULL;
+	return 0;
+}
+
 /*
  * FIXME: Functions below should be implemented properly.
  *        For now, just add stubs for NO_NEWT=1 build.
-- 
1.7.9.2


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 5/7] perf ui/gtk: Add GTK statusbar widget to browser window
  2012-05-29  4:22 [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end (v3) Namhyung Kim
                   ` (3 preceding siblings ...)
  2012-05-29  4:22 ` [PATCH 4/7] perf ui/gtk: Introduce struct perf_gtk_context Namhyung Kim
@ 2012-05-29  4:23 ` Namhyung Kim
  2012-06-20 16:47   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2012-05-29  4:23 ` [PATCH 6/7] perf ui/gtk: Add GTK info_bar " Namhyung Kim
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: Namhyung Kim @ 2012-05-29  4:23 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Pekka Enberg, LKML

Add statusbar widget to display non-critical messages
at the bottom of the window. This can be used for
showing a status change, warning or help message.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 tools/perf/ui/gtk/browser.c |   26 +++++++++++++++++++++++++-
 tools/perf/ui/gtk/gtk.h     |    2 ++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index 33ab1aee3472..ece360db8658 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -122,13 +122,30 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 	gtk_container_add(GTK_CONTAINER(window), view);
 }
 
+static GtkWidget *perf_gtk__setup_statusbar(void)
+{
+	GtkWidget *stbar;
+	unsigned ctxid;
+
+	stbar = gtk_statusbar_new();
+
+	ctxid = gtk_statusbar_get_context_id(GTK_STATUSBAR(stbar),
+					     "perf report");
+	pgctx->statbar = stbar;
+	pgctx->statbar_ctx_id = ctxid;
+
+	return stbar;
+}
+
 int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 				  const char *help __used,
 				  void (*timer) (void *arg)__used,
 				  void *arg __used, int delay_secs __used)
 {
 	struct perf_evsel *pos;
+	GtkWidget *vbox;
 	GtkWidget *notebook;
+	GtkWidget *statbar;
 	GtkWidget *window;
 
 	signal(SIGSEGV, perf_gtk__signal);
@@ -147,6 +164,8 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 	if (!pgctx)
 		return -1;
 
+	vbox = gtk_vbox_new(FALSE, 0);
+
 	notebook = gtk_notebook_new();
 
 	list_for_each_entry(pos, &evlist->entries, node) {
@@ -168,7 +187,12 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 		gtk_notebook_append_page(GTK_NOTEBOOK(notebook), scrolled_window, tab_label);
 	}
 
-	gtk_container_add(GTK_CONTAINER(window), notebook);
+	gtk_box_pack_start(GTK_BOX(vbox), notebook, TRUE, TRUE, 0);
+
+	statbar = perf_gtk__setup_statusbar();
+	gtk_box_pack_start(GTK_BOX(vbox), statbar, FALSE, FALSE, 0);
+
+	gtk_container_add(GTK_CONTAINER(window), vbox);
 
 	gtk_widget_show_all(window);
 
diff --git a/tools/perf/ui/gtk/gtk.h b/tools/perf/ui/gtk/gtk.h
index 34fbca6d48a2..206167868c5f 100644
--- a/tools/perf/ui/gtk/gtk.h
+++ b/tools/perf/ui/gtk/gtk.h
@@ -10,6 +10,8 @@
 
 struct perf_gtk_context {
 	GtkWidget *main_window;
+	GtkWidget *statbar;
+	guint statbar_ctx_id;
 };
 
 extern struct perf_gtk_context *pgctx;
-- 
1.7.9.2


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 6/7] perf ui/gtk: Add GTK info_bar widget to browser window
  2012-05-29  4:22 [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end (v3) Namhyung Kim
                   ` (4 preceding siblings ...)
  2012-05-29  4:23 ` [PATCH 5/7] perf ui/gtk: Add GTK statusbar widget to browser window Namhyung Kim
@ 2012-05-29  4:23 ` Namhyung Kim
  2012-06-20 16:48   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2012-05-29  4:23 ` [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops Namhyung Kim
  2012-05-30  1:31 ` [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end (v3) Arnaldo Carvalho de Melo
  7 siblings, 1 reply; 22+ messages in thread
From: Namhyung Kim @ 2012-05-29  4:23 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Pekka Enberg, LKML

The GtkInfoBar is a modern UI component to display messages
without bothering the main window. It'll be used for showing
a warning message.

As the GtkInfoBar requires 2.18 (or newer) version of GTK+
library, add availability check to Makefile too.

Suggested-by: Sunjin Yang <fan4326@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 tools/perf/Makefile                 |    3 +++
 tools/perf/config/feature-tests.mak |   13 +++++++++++++
 tools/perf/ui/gtk/browser.c         |   33 +++++++++++++++++++++++++++++++++
 tools/perf/ui/gtk/gtk.h             |   12 ++++++++++++
 4 files changed, 61 insertions(+)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 7034b3c70d42..14baf7b22647 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -523,6 +523,9 @@ else
 		msg := $(warning GTK2 not found, disables GTK2 support. Please install gtk2-devel or libgtk2.0-dev);
 		BASIC_CFLAGS += -DNO_GTK2_SUPPORT
 	else
+		ifeq ($(call try-cc,$(SOURCE_GTK2_INFOBAR),$(FLAGS_GTK2)),y)
+			BASIC_CFLAGS += -DHAVE_GTK_INFO_BAR
+		endif
 		BASIC_CFLAGS += $(shell pkg-config --cflags gtk+-2.0)
 		EXTLIBS += $(shell pkg-config --libs gtk+-2.0)
 		LIB_OBJS += $(OUTPUT)ui/gtk/browser.o
diff --git a/tools/perf/config/feature-tests.mak b/tools/perf/config/feature-tests.mak
index d9084e03ce56..6c18785a6417 100644
--- a/tools/perf/config/feature-tests.mak
+++ b/tools/perf/config/feature-tests.mak
@@ -78,6 +78,19 @@ int main(int argc, char *argv[])
         return 0;
 }
 endef
+
+define SOURCE_GTK2_INFOBAR
+#pragma GCC diagnostic ignored \"-Wstrict-prototypes\"
+#include <gtk/gtk.h>
+#pragma GCC diagnostic error \"-Wstrict-prototypes\"
+
+int main(void)
+{
+	gtk_info_bar_new();
+
+	return 0;
+}
+endef
 endif
 
 ifndef NO_LIBPERL
diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index ece360db8658..fd41e8d10337 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -122,6 +122,34 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 	gtk_container_add(GTK_CONTAINER(window), view);
 }
 
+#ifdef HAVE_GTK_INFO_BAR
+static GtkWidget *perf_gtk__setup_info_bar(void)
+{
+	GtkWidget *info_bar;
+	GtkWidget *label;
+	GtkWidget *content_area;
+
+	info_bar = gtk_info_bar_new();
+	gtk_widget_set_no_show_all(info_bar, TRUE);
+
+	label = gtk_label_new("");
+	gtk_widget_show(label);
+
+	content_area = gtk_info_bar_get_content_area(GTK_INFO_BAR(info_bar));
+	gtk_container_add(GTK_CONTAINER(content_area), label);
+
+	gtk_info_bar_add_button(GTK_INFO_BAR(info_bar), GTK_STOCK_OK,
+				GTK_RESPONSE_OK);
+	g_signal_connect(info_bar, "response",
+			 G_CALLBACK(gtk_widget_hide), NULL);
+
+	pgctx->info_bar = info_bar;
+	pgctx->message_label = label;
+
+	return info_bar;
+}
+#endif
+
 static GtkWidget *perf_gtk__setup_statusbar(void)
 {
 	GtkWidget *stbar;
@@ -145,6 +173,7 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 	struct perf_evsel *pos;
 	GtkWidget *vbox;
 	GtkWidget *notebook;
+	GtkWidget *info_bar;
 	GtkWidget *statbar;
 	GtkWidget *window;
 
@@ -189,6 +218,10 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 
 	gtk_box_pack_start(GTK_BOX(vbox), notebook, TRUE, TRUE, 0);
 
+	info_bar = perf_gtk__setup_info_bar();
+	if (info_bar)
+		gtk_box_pack_start(GTK_BOX(vbox), info_bar, FALSE, FALSE, 0);
+
 	statbar = perf_gtk__setup_statusbar();
 	gtk_box_pack_start(GTK_BOX(vbox), statbar, FALSE, FALSE, 0);
 
diff --git a/tools/perf/ui/gtk/gtk.h b/tools/perf/ui/gtk/gtk.h
index 206167868c5f..a4d0f2b4a2dc 100644
--- a/tools/perf/ui/gtk/gtk.h
+++ b/tools/perf/ui/gtk/gtk.h
@@ -10,6 +10,11 @@
 
 struct perf_gtk_context {
 	GtkWidget *main_window;
+
+#ifdef HAVE_GTK_INFO_BAR
+	GtkWidget *info_bar;
+	GtkWidget *message_label;
+#endif
 	GtkWidget *statbar;
 	guint statbar_ctx_id;
 };
@@ -24,4 +29,11 @@ static inline bool perf_gtk__is_active_context(struct perf_gtk_context *ctx)
 struct perf_gtk_context *perf_gtk__activate_context(GtkWidget *window);
 int perf_gtk__deactivate_context(struct perf_gtk_context **ctx);
 
+#ifndef HAVE_GTK_INFO_BAR
+static inline GtkWidget *perf_gtk__setup_info_bar(void)
+{
+	return NULL;
+}
+#endif
+
 #endif /* _PERF_GTK_H_ */
-- 
1.7.9.2


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops
  2012-05-29  4:22 [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end (v3) Namhyung Kim
                   ` (5 preceding siblings ...)
  2012-05-29  4:23 ` [PATCH 6/7] perf ui/gtk: Add GTK info_bar " Namhyung Kim
@ 2012-05-29  4:23 ` Namhyung Kim
  2012-06-20 16:49   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2012-05-30  1:31 ` [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end (v3) Arnaldo Carvalho de Melo
  7 siblings, 1 reply; 22+ messages in thread
From: Namhyung Kim @ 2012-05-29  4:23 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Pekka Enberg, LKML

Define and use perf_gtk_eops to provide a GTK2 message
dialog for error reporting and a info_bar for warning.

As GtkInfoBar requires recent GTK+ libraries, provides
a fallback implementation using statusbar widget too.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 tools/perf/ui/gtk/setup.c |    5 +++
 tools/perf/ui/gtk/util.c  |   86 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+)

diff --git a/tools/perf/ui/gtk/setup.c b/tools/perf/ui/gtk/setup.c
index 829529957766..92879ce61e2f 100644
--- a/tools/perf/ui/gtk/setup.c
+++ b/tools/perf/ui/gtk/setup.c
@@ -1,12 +1,17 @@
 #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(&perf_gtk_eops);
 	return gtk_init_check(NULL, NULL) ? 0 : -1;
 }
 
 void perf_gtk__exit(bool wait_for_ok __used)
 {
+	perf_error__unregister(&perf_gtk_eops);
 	gtk_main_quit();
 }
diff --git a/tools/perf/ui/gtk/util.c b/tools/perf/ui/gtk/util.c
index 6fe13fdc513e..0ead373c0dfb 100644
--- a/tools/perf/ui/gtk/util.c
+++ b/tools/perf/ui/gtk/util.c
@@ -2,6 +2,8 @@
 #include "../../util/debug.h"
 #include "gtk.h"
 
+#include <string.h>
+
 
 struct perf_gtk_context *pgctx;
 
@@ -26,6 +28,90 @@ int perf_gtk__deactivate_context(struct perf_gtk_context **ctx)
 	return 0;
 }
 
+static int perf_gtk__error(const char *format, va_list args)
+{
+	char *msg;
+	GtkWidget *dialog;
+
+	if (!perf_gtk__is_active_context(pgctx) ||
+	    vasprintf(&msg, format, args) < 0) {
+		fprintf(stderr, "Error:\n");
+		vfprintf(stderr, format, args);
+		fprintf(stderr, "\n");
+		return -1;
+	}
+
+	dialog = gtk_message_dialog_new_with_markup(GTK_WINDOW(pgctx->main_window),
+					GTK_DIALOG_DESTROY_WITH_PARENT,
+					GTK_MESSAGE_ERROR,
+					GTK_BUTTONS_CLOSE,
+					"<b>Error</b>\n\n%s", msg);
+	gtk_dialog_run(GTK_DIALOG(dialog));
+
+	gtk_widget_destroy(dialog);
+	free(msg);
+	return 0;
+}
+
+#ifdef HAVE_GTK_INFO_BAR
+static int perf_gtk__warning_info_bar(const char *format, va_list args)
+{
+	char *msg;
+
+	if (!perf_gtk__is_active_context(pgctx) ||
+	    vasprintf(&msg, format, args) < 0) {
+		fprintf(stderr, "Warning:\n");
+		vfprintf(stderr, format, args);
+		fprintf(stderr, "\n");
+		return -1;
+	}
+
+	gtk_label_set_text(GTK_LABEL(pgctx->message_label), msg);
+	gtk_info_bar_set_message_type(GTK_INFO_BAR(pgctx->info_bar),
+				      GTK_MESSAGE_WARNING);
+	gtk_widget_show(pgctx->info_bar);
+
+	free(msg);
+	return 0;
+}
+#else
+static int perf_gtk__warning_statusbar(const char *format, va_list args)
+{
+	char *msg, *p;
+
+	if (!perf_gtk__is_active_context(pgctx) ||
+	    vasprintf(&msg, format, args) < 0) {
+		fprintf(stderr, "Warning:\n");
+		vfprintf(stderr, format, args);
+		fprintf(stderr, "\n");
+		return -1;
+	}
+
+	gtk_statusbar_pop(GTK_STATUSBAR(pgctx->statbar),
+			  pgctx->statbar_ctx_id);
+
+	/* Only first line can be displayed */
+	p = strchr(msg, '\n');
+	if (p)
+		*p = '\0';
+
+	gtk_statusbar_push(GTK_STATUSBAR(pgctx->statbar),
+			   pgctx->statbar_ctx_id, msg);
+
+	free(msg);
+	return 0;
+}
+#endif
+
+struct perf_error_ops perf_gtk_eops = {
+	.error		= perf_gtk__error,
+#ifdef HAVE_GTK_INFO_BAR
+	.warning	= perf_gtk__warning_info_bar,
+#else
+	.warning	= perf_gtk__warning_statusbar,
+#endif
+};
+
 /*
  * FIXME: Functions below should be implemented properly.
  *        For now, just add stubs for NO_NEWT=1 build.
-- 
1.7.9.2


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end (v3)
  2012-05-29  4:22 [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end (v3) Namhyung Kim
                   ` (6 preceding siblings ...)
  2012-05-29  4:23 ` [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops Namhyung Kim
@ 2012-05-30  1:31 ` Arnaldo Carvalho de Melo
  2012-05-30  6:19   ` Pekka Enberg
  7 siblings, 1 reply; 22+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-30  1:31 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Pekka Enberg, LKML

Em Tue, May 29, 2012 at 01:22:55PM +0900, Namhyung Kim escreveu:
> Hi,
> 
> This patchset implements ui__warning/error() functions for GTK2.
> Patch 1 is a resend (which forgot to cc LKML) and independent of
> this series. Patch 2 converts invocations of ui__warning followed
> by exit (or similar effect) into ui__error.

Pekka, all well with you? Acked-by?

- Arnaldo

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end (v3)
  2012-05-30  1:31 ` [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end (v3) Arnaldo Carvalho de Melo
@ 2012-05-30  6:19   ` Pekka Enberg
  0 siblings, 0 replies; 22+ messages in thread
From: Pekka Enberg @ 2012-05-30  6:19 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML

On Wed, May 30, 2012 at 4:31 AM, Arnaldo Carvalho de Melo
<acme@ghostprotocols.net> wrote:
> Em Tue, May 29, 2012 at 01:22:55PM +0900, Namhyung Kim escreveu:
>> Hi,
>>
>> This patchset implements ui__warning/error() functions for GTK2.
>> Patch 1 is a resend (which forgot to cc LKML) and independent of
>> this series. Patch 2 converts invocations of ui__warning followed
>> by exit (or similar effect) into ui__error.
>
> Pekka, all well with you? Acked-by?

Looks good to me!

Acked-by: Pekka Enberg <penberg@kernel.org>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [tip:perf/urgent] perf tools: Convert critical messages to ui__error()
  2012-05-29  4:22 ` [PATCH 2/7] perf tools: Convert critical messages to ui__error() Namhyung Kim
@ 2012-05-30  7:49   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 22+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-05-30  7:49 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, penberg,
	namhyung, tglx

Commit-ID:  3780f4883b2f3319afe88bf3ddc73ef426851d49
Gitweb:     http://git.kernel.org/tip/3780f4883b2f3319afe88bf3ddc73ef426851d49
Author:     Namhyung Kim <namhyung@gmail.com>
AuthorDate: Tue, 29 May 2012 13:22:57 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 29 May 2012 11:53:42 -0300

perf tools: Convert critical messages to ui__error()

There were places where use ui__warning (or even fprintf) to show
critical messages. This patch converts them to ui__error so that the
front-end code can implement appropriate behavior.

Signed-off-by: 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/1338265382-6872-3-git-send-email-namhyung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-annotate.c |    2 +-
 tools/perf/builtin-record.c   |   14 +++++++-------
 tools/perf/builtin-report.c   |   14 ++++++--------
 tools/perf/builtin-top.c      |   22 +++++++++++-----------
 4 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 806e0a2..67522cf 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -215,7 +215,7 @@ static int __cmd_annotate(struct perf_annotate *ann)
 	}
 
 	if (total_nr_samples == 0) {
-		ui__warning("The %s file has no samples!\n", session->filename);
+		ui__error("The %s file has no samples!\n", session->filename);
 		goto out_delete;
 	}
 out_delete:
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index e5cb084..f95840d 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -264,7 +264,7 @@ try_again:
 			}
 
 			if (err == ENOENT) {
-				ui__warning("The %s event is not supported.\n",
+				ui__error("The %s event is not supported.\n",
 					    event_name(pos));
 				exit(EXIT_FAILURE);
 			}
@@ -858,8 +858,8 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		usage_with_options(record_usage, record_options);
 
 	if (rec->force && rec->append_file) {
-		fprintf(stderr, "Can't overwrite and append at the same time."
-				" You need to choose between -f and -A");
+		ui__error("Can't overwrite and append at the same time."
+			  " You need to choose between -f and -A");
 		usage_with_options(record_usage, record_options);
 	} else if (rec->append_file) {
 		rec->write_mode = WRITE_APPEND;
@@ -868,8 +868,8 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 	}
 
 	if (nr_cgroups && !rec->opts.target.system_wide) {
-		fprintf(stderr, "cgroup monitoring only available in"
-			" system-wide mode\n");
+		ui__error("cgroup monitoring only available in"
+			  " system-wide mode\n");
 		usage_with_options(record_usage, record_options);
 	}
 
@@ -905,7 +905,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 		int saved_errno = errno;
 
 		perf_target__strerror(&rec->opts.target, err, errbuf, BUFSIZ);
-		ui__warning("%s", errbuf);
+		ui__error("%s", errbuf);
 
 		err = -saved_errno;
 		goto out_free_fd;
@@ -933,7 +933,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
 	else if (rec->opts.freq) {
 		rec->opts.default_interval = rec->opts.freq;
 	} else {
-		fprintf(stderr, "frequency and count are zero, aborting\n");
+		ui__error("frequency and count are zero, aborting\n");
 		err = -EINVAL;
 		goto out_free_fd;
 	}
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index d58e414..8c767c6 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -251,13 +251,13 @@ static int perf_report__setup_sample_type(struct perf_report *rep)
 
 	if (!(self->sample_type & PERF_SAMPLE_CALLCHAIN)) {
 		if (sort__has_parent) {
-			ui__warning("Selected --sort parent, but no "
+			ui__error("Selected --sort parent, but no "
 				    "callchain data. Did you call "
 				    "'perf record' without -g?\n");
 			return -EINVAL;
 		}
 		if (symbol_conf.use_callchain) {
-			ui__warning("Selected -g but no callchain data. Did "
+			ui__error("Selected -g but no callchain data. Did "
 				    "you call 'perf record' without -g?\n");
 			return -1;
 		}
@@ -266,17 +266,15 @@ static int perf_report__setup_sample_type(struct perf_report *rep)
 		   !symbol_conf.use_callchain) {
 			symbol_conf.use_callchain = true;
 			if (callchain_register_param(&callchain_param) < 0) {
-				ui__warning("Can't register callchain "
-					    "params.\n");
+				ui__error("Can't register callchain params.\n");
 				return -EINVAL;
 			}
 	}
 
 	if (sort__branch_mode == 1) {
 		if (!(self->sample_type & PERF_SAMPLE_BRANCH_STACK)) {
-			fprintf(stderr, "selected -b but no branch data."
-					" Did you call perf record without"
-					" -b?\n");
+			ui__error("Selected -b but no branch data. "
+				  "Did you call perf record without -b?\n");
 			return -1;
 		}
 	}
@@ -420,7 +418,7 @@ static int __cmd_report(struct perf_report *rep)
 	}
 
 	if (nr_samples == 0) {
-		ui__warning("The %s file has no samples!\n", session->filename);
+		ui__error("The %s file has no samples!\n", session->filename);
 		goto out_delete;
 	}
 
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index d4a5f9b..871b540 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -959,16 +959,16 @@ try_again:
 			}
 
 			if (err == ENOENT) {
-				ui__warning("The %s event is not supported.\n",
+				ui__error("The %s event is not supported.\n",
 					    event_name(counter));
 				goto out_err;
 			} else if (err == EMFILE) {
-				ui__warning("Too many events are opened.\n"
+				ui__error("Too many events are opened.\n"
 					    "Try again after reducing the number of events\n");
 				goto out_err;
 			}
 
-			ui__warning("The sys_perf_event_open() syscall "
+			ui__error("The sys_perf_event_open() syscall "
 				    "returned with %d (%s).  /bin/dmesg "
 				    "may provide additional information.\n"
 				    "No CONFIG_PERF_EVENTS=y kernel support "
@@ -978,7 +978,7 @@ try_again:
 	}
 
 	if (perf_evlist__mmap(evlist, top->mmap_pages, false) < 0) {
-		ui__warning("Failed to mmap with %d (%s)\n",
+		ui__error("Failed to mmap with %d (%s)\n",
 			    errno, strerror(errno));
 		goto out_err;
 	}
@@ -994,12 +994,12 @@ static int perf_top__setup_sample_type(struct perf_top *top)
 {
 	if (!top->sort_has_symbols) {
 		if (symbol_conf.use_callchain) {
-			ui__warning("Selected -g but \"sym\" not present in --sort/-s.");
+			ui__error("Selected -g but \"sym\" not present in --sort/-s.");
 			return -EINVAL;
 		}
 	} else if (!top->dont_use_callchains && callchain_param.mode != CHAIN_NONE) {
 		if (callchain_register_param(&callchain_param) < 0) {
-			ui__warning("Can't register callchain params.\n");
+			ui__error("Can't register callchain params.\n");
 			return -EINVAL;
 		}
 	}
@@ -1041,7 +1041,7 @@ static int __cmd_top(struct perf_top *top)
 
 	if (pthread_create(&thread, NULL, (use_browser > 0 ? display_thread_tui :
 							    display_thread), top)) {
-		printf("Could not create display thread.\n");
+		ui__error("Could not create display thread.\n");
 		exit(-1);
 	}
 
@@ -1050,7 +1050,7 @@ static int __cmd_top(struct perf_top *top)
 
 		param.sched_priority = top->realtime_prio;
 		if (sched_setscheduler(0, SCHED_FIFO, &param)) {
-			printf("Could not set realtime priority.\n");
+			ui__error("Could not set realtime priority.\n");
 			exit(-1);
 		}
 	}
@@ -1274,7 +1274,7 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 		int saved_errno = errno;
 
 		perf_target__strerror(&top.target, status, errbuf, BUFSIZ);
-		ui__warning("%s", errbuf);
+		ui__error("%s", errbuf);
 
 		status = -saved_errno;
 		goto out_delete_evlist;
@@ -1288,7 +1288,7 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 
 	if (!top.evlist->nr_entries &&
 	    perf_evlist__add_default(top.evlist) < 0) {
-		pr_err("Not enough memory for event selector list\n");
+		ui__error("Not enough memory for event selector list\n");
 		return -ENOMEM;
 	}
 
@@ -1305,7 +1305,7 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 	else if (top.freq) {
 		top.default_interval = top.freq;
 	} else {
-		fprintf(stderr, "frequency and count are zero, aborting\n");
+		ui__error("frequency and count are zero, aborting\n");
 		exit(EXIT_FAILURE);
 	}
 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [tip:perf/core] perf ui: Introduce struct perf_error_ops
  2012-05-29  4:22 ` [PATCH 3/7] perf ui: Introduce struct perf_error_ops Namhyung Kim
@ 2012-06-20 16:45   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 22+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-06-20 16:45 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, penberg,
	namhyung.kim, tglx

Commit-ID:  ba47a142d9f9b84e0464a11b7a067e5ad95c5d4b
Gitweb:     http://git.kernel.org/tip/ba47a142d9f9b84e0464a11b7a067e5ad95c5d4b
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Tue, 29 May 2012 13:22:58 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 19 Jun 2012 13:06:18 -0300

perf ui: Introduce struct perf_error_ops

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>
Acked-by: Pekka Enberg <penberg@kernel.org>
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/1338265382-6872-4-git-send-email-namhyung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.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 |   49 +++----
 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, 135 insertions(+), 254 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 0eee64c..c0ee917 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -503,6 +503,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
@@ -526,9 +527,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 0000000..a727fe3
--- /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 d33e943..e813c1d 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(&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(&perf_tui_eops);
 }
diff --git a/tools/perf/ui/util.c b/tools/perf/ui/tui/util.c
similarity index 86%
copy from tools/perf/ui/util.c
copy to tools/perf/ui/tui/util.c
index ad4374a..092902e 100644
--- a/tools/perf/ui/util.c
+++ b/tools/perf/ui/tui/util.c
@@ -1,17 +1,17 @@
-#include "../util.h"
+#include "../../util/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"
+#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)
@@ -208,11 +208,11 @@ 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)
+static int __ui__warning(const char *title, const char *format, va_list args)
 {
 	char *s;
 
-	if (use_browser > 0 && vasprintf(&s, format, args) > 0) {
+	if (vasprintf(&s, format, args) > 0) {
 		int key;
 
 		pthread_mutex_lock(&ui__lock);
@@ -222,29 +222,22 @@ int __ui__warning(const char *title, const char *format, va_list args)
 		return key;
 	}
 
-	fprintf(stderr, "%s:\n", title);
+	fprintf(stderr, "%s\n", title);
 	vfprintf(stderr, format, args);
 	return K_ESC;
 }
 
-int ui__warning(const char *format, ...)
+static int perf_tui__error(const char *format, va_list args)
 {
-	int key;
-	va_list args;
-
-	va_start(args, format);
-	key = __ui__warning("Warning", format, args);
-	va_end(args);
-	return key;
+	return __ui__warning("Error:", format, args);
 }
 
-int ui__error(const char *format, ...)
+static int perf_tui__warning(const char *format, va_list args)
 {
-	int key;
-	va_list args;
-
-	va_start(args, format);
-	key = __ui__warning("Error", format, args);
-	va_end(args);
-	return key;
+	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 ad4374a..4f98977 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 - 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(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 - Unregister error logging functions
+ * @eops: The pointer to error logging function struct
+ *
+ * Unregister already registered error logging functions.
+ */
+int perf_error__unregister(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 2d1738b..361f08c 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(struct perf_error_ops *eops);
+int perf_error__unregister(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 efb1fce..4dfe0bb 100644
--- a/tools/perf/util/debug.c
+++ b/tools/perf/util/debug.c
@@ -47,7 +47,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 6bebe7f..015c91d 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(struct perf_error_ops *eops __used)
+{
+	return 0;
+}
+
+static inline int
+perf_error__unregister(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);

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [tip:perf/core] perf ui/gtk: Introduce struct perf_gtk_context
  2012-05-29  4:22 ` [PATCH 4/7] perf ui/gtk: Introduce struct perf_gtk_context Namhyung Kim
@ 2012-06-20 16:46   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 22+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-06-20 16:46 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, penberg,
	namhyung, tglx

Commit-ID:  42ab68a35ffee04700648ec42c9507145a66837d
Gitweb:     http://git.kernel.org/tip/42ab68a35ffee04700648ec42c9507145a66837d
Author:     Namhyung Kim <namhyung@gmail.com>
AuthorDate: Tue, 29 May 2012 13:22:59 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 19 Jun 2012 13:06:18 -0300

perf ui/gtk: Introduce struct perf_gtk_context

The struct perf_gtk_context is for tracking current state of GTK window
and/or other things. This is a preparation of next changes.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Acked-by: Pekka Enberg <penberg@kernel.org>
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/1338265382-6872-5-git-send-email-namhyung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/gtk/browser.c |    8 +++++++-
 tools/perf/ui/gtk/gtk.h     |   17 +++++++++++++++++
 tools/perf/ui/gtk/util.c    |   23 +++++++++++++++++++++++
 3 files changed, 47 insertions(+), 1 deletions(-)

diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index 0656c38..33ab1ae 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -11,8 +11,8 @@
 
 static void perf_gtk__signal(int sig)
 {
+	perf_gtk__exit(false);
 	psignal(sig, "perf");
-	gtk_main_quit();
 }
 
 static void perf_gtk__resize_window(GtkWidget *window)
@@ -143,6 +143,10 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 
 	g_signal_connect(window, "delete_event", gtk_main_quit, NULL);
 
+	pgctx = perf_gtk__activate_context(window);
+	if (!pgctx)
+		return -1;
+
 	notebook = gtk_notebook_new();
 
 	list_for_each_entry(pos, &evlist->entries, node) {
@@ -174,5 +178,7 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 
 	gtk_main();
 
+	perf_gtk__deactivate_context(&pgctx);
+
 	return 0;
 }
diff --git a/tools/perf/ui/gtk/gtk.h b/tools/perf/ui/gtk/gtk.h
index 75177ee..34fbca6 100644
--- a/tools/perf/ui/gtk/gtk.h
+++ b/tools/perf/ui/gtk/gtk.h
@@ -1,8 +1,25 @@
 #ifndef _PERF_GTK_H_
 #define _PERF_GTK_H_ 1
 
+#include <stdbool.h>
+
 #pragma GCC diagnostic ignored "-Wstrict-prototypes"
 #include <gtk/gtk.h>
 #pragma GCC diagnostic error "-Wstrict-prototypes"
 
+
+struct perf_gtk_context {
+	GtkWidget *main_window;
+};
+
+extern struct perf_gtk_context *pgctx;
+
+static inline bool perf_gtk__is_active_context(struct perf_gtk_context *ctx)
+{
+	return ctx && ctx->main_window;
+}
+
+struct perf_gtk_context *perf_gtk__activate_context(GtkWidget *window);
+int perf_gtk__deactivate_context(struct perf_gtk_context **ctx);
+
 #endif /* _PERF_GTK_H_ */
diff --git a/tools/perf/ui/gtk/util.c b/tools/perf/ui/gtk/util.c
index a727fe3..6fe13fd 100644
--- a/tools/perf/ui/gtk/util.c
+++ b/tools/perf/ui/gtk/util.c
@@ -3,6 +3,29 @@
 #include "gtk.h"
 
 
+struct perf_gtk_context *pgctx;
+
+struct perf_gtk_context *perf_gtk__activate_context(GtkWidget *window)
+{
+	struct perf_gtk_context *ctx;
+
+	ctx = malloc(sizeof(*pgctx));
+	if (ctx)
+		ctx->main_window = window;
+
+	return ctx;
+}
+
+int perf_gtk__deactivate_context(struct perf_gtk_context **ctx)
+{
+	if (!perf_gtk__is_active_context(*ctx))
+		return -1;
+
+	free(*ctx);
+	*ctx = NULL;
+	return 0;
+}
+
 /*
  * FIXME: Functions below should be implemented properly.
  *        For now, just add stubs for NO_NEWT=1 build.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [tip:perf/core] perf ui/gtk: Add GTK statusbar widget to browser window
  2012-05-29  4:23 ` [PATCH 5/7] perf ui/gtk: Add GTK statusbar widget to browser window Namhyung Kim
@ 2012-06-20 16:47   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 22+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-06-20 16:47 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, penberg,
	namhyung, tglx

Commit-ID:  b4418c6848dcd56cc90625dcfaef7cb019492233
Gitweb:     http://git.kernel.org/tip/b4418c6848dcd56cc90625dcfaef7cb019492233
Author:     Namhyung Kim <namhyung@gmail.com>
AuthorDate: Tue, 29 May 2012 13:23:00 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 19 Jun 2012 13:06:19 -0300

perf ui/gtk: Add GTK statusbar widget to browser window

Add statusbar widget to display non-critical messages at the bottom of
the window. This can be used for showing a status change, warning or
help message.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Acked-by: Pekka Enberg <penberg@kernel.org>
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/1338265382-6872-6-git-send-email-namhyung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/gtk/browser.c |   26 +++++++++++++++++++++++++-
 tools/perf/ui/gtk/gtk.h     |    2 ++
 2 files changed, 27 insertions(+), 1 deletions(-)

diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index 33ab1ae..ece360d 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -122,13 +122,30 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 	gtk_container_add(GTK_CONTAINER(window), view);
 }
 
+static GtkWidget *perf_gtk__setup_statusbar(void)
+{
+	GtkWidget *stbar;
+	unsigned ctxid;
+
+	stbar = gtk_statusbar_new();
+
+	ctxid = gtk_statusbar_get_context_id(GTK_STATUSBAR(stbar),
+					     "perf report");
+	pgctx->statbar = stbar;
+	pgctx->statbar_ctx_id = ctxid;
+
+	return stbar;
+}
+
 int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 				  const char *help __used,
 				  void (*timer) (void *arg)__used,
 				  void *arg __used, int delay_secs __used)
 {
 	struct perf_evsel *pos;
+	GtkWidget *vbox;
 	GtkWidget *notebook;
+	GtkWidget *statbar;
 	GtkWidget *window;
 
 	signal(SIGSEGV, perf_gtk__signal);
@@ -147,6 +164,8 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 	if (!pgctx)
 		return -1;
 
+	vbox = gtk_vbox_new(FALSE, 0);
+
 	notebook = gtk_notebook_new();
 
 	list_for_each_entry(pos, &evlist->entries, node) {
@@ -168,7 +187,12 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 		gtk_notebook_append_page(GTK_NOTEBOOK(notebook), scrolled_window, tab_label);
 	}
 
-	gtk_container_add(GTK_CONTAINER(window), notebook);
+	gtk_box_pack_start(GTK_BOX(vbox), notebook, TRUE, TRUE, 0);
+
+	statbar = perf_gtk__setup_statusbar();
+	gtk_box_pack_start(GTK_BOX(vbox), statbar, FALSE, FALSE, 0);
+
+	gtk_container_add(GTK_CONTAINER(window), vbox);
 
 	gtk_widget_show_all(window);
 
diff --git a/tools/perf/ui/gtk/gtk.h b/tools/perf/ui/gtk/gtk.h
index 34fbca6..2061678 100644
--- a/tools/perf/ui/gtk/gtk.h
+++ b/tools/perf/ui/gtk/gtk.h
@@ -10,6 +10,8 @@
 
 struct perf_gtk_context {
 	GtkWidget *main_window;
+	GtkWidget *statbar;
+	guint statbar_ctx_id;
 };
 
 extern struct perf_gtk_context *pgctx;

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [tip:perf/core] perf ui/gtk: Add GTK info_bar widget to browser window
  2012-05-29  4:23 ` [PATCH 6/7] perf ui/gtk: Add GTK info_bar " Namhyung Kim
@ 2012-06-20 16:48   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 22+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-06-20 16:48 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, penberg,
	namhyung, fan4326, tglx

Commit-ID:  a6b702c117f839023814c1e03453c701d26de522
Gitweb:     http://git.kernel.org/tip/a6b702c117f839023814c1e03453c701d26de522
Author:     Namhyung Kim <namhyung@gmail.com>
AuthorDate: Tue, 29 May 2012 13:23:01 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 19 Jun 2012 13:06:19 -0300

perf ui/gtk: Add GTK info_bar widget to browser window

The GtkInfoBar is a modern UI component to display messages without
bothering the main window. It'll be used for showing a warning message.

As the GtkInfoBar requires 2.18 (or newer) version of GTK+ library, add
availability check to Makefile too.

Suggested-by: Sunjin Yang <fan4326@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Acked-by: Pekka Enberg <penberg@kernel.org>
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/1338265382-6872-7-git-send-email-namhyung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile                 |    3 +++
 tools/perf/config/feature-tests.mak |   13 +++++++++++++
 tools/perf/ui/gtk/browser.c         |   33 +++++++++++++++++++++++++++++++++
 tools/perf/ui/gtk/gtk.h             |   12 ++++++++++++
 4 files changed, 61 insertions(+), 0 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index c0ee917..d698c11 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -523,6 +523,9 @@ else
 		msg := $(warning GTK2 not found, disables GTK2 support. Please install gtk2-devel or libgtk2.0-dev);
 		BASIC_CFLAGS += -DNO_GTK2_SUPPORT
 	else
+		ifeq ($(call try-cc,$(SOURCE_GTK2_INFOBAR),$(FLAGS_GTK2)),y)
+			BASIC_CFLAGS += -DHAVE_GTK_INFO_BAR
+		endif
 		BASIC_CFLAGS += $(shell pkg-config --cflags gtk+-2.0)
 		EXTLIBS += $(shell pkg-config --libs gtk+-2.0)
 		LIB_OBJS += $(OUTPUT)ui/gtk/browser.o
diff --git a/tools/perf/config/feature-tests.mak b/tools/perf/config/feature-tests.mak
index d9084e0..6c18785 100644
--- a/tools/perf/config/feature-tests.mak
+++ b/tools/perf/config/feature-tests.mak
@@ -78,6 +78,19 @@ int main(int argc, char *argv[])
         return 0;
 }
 endef
+
+define SOURCE_GTK2_INFOBAR
+#pragma GCC diagnostic ignored \"-Wstrict-prototypes\"
+#include <gtk/gtk.h>
+#pragma GCC diagnostic error \"-Wstrict-prototypes\"
+
+int main(void)
+{
+	gtk_info_bar_new();
+
+	return 0;
+}
+endef
 endif
 
 ifndef NO_LIBPERL
diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index ece360d..fd41e8d 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -122,6 +122,34 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 	gtk_container_add(GTK_CONTAINER(window), view);
 }
 
+#ifdef HAVE_GTK_INFO_BAR
+static GtkWidget *perf_gtk__setup_info_bar(void)
+{
+	GtkWidget *info_bar;
+	GtkWidget *label;
+	GtkWidget *content_area;
+
+	info_bar = gtk_info_bar_new();
+	gtk_widget_set_no_show_all(info_bar, TRUE);
+
+	label = gtk_label_new("");
+	gtk_widget_show(label);
+
+	content_area = gtk_info_bar_get_content_area(GTK_INFO_BAR(info_bar));
+	gtk_container_add(GTK_CONTAINER(content_area), label);
+
+	gtk_info_bar_add_button(GTK_INFO_BAR(info_bar), GTK_STOCK_OK,
+				GTK_RESPONSE_OK);
+	g_signal_connect(info_bar, "response",
+			 G_CALLBACK(gtk_widget_hide), NULL);
+
+	pgctx->info_bar = info_bar;
+	pgctx->message_label = label;
+
+	return info_bar;
+}
+#endif
+
 static GtkWidget *perf_gtk__setup_statusbar(void)
 {
 	GtkWidget *stbar;
@@ -145,6 +173,7 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 	struct perf_evsel *pos;
 	GtkWidget *vbox;
 	GtkWidget *notebook;
+	GtkWidget *info_bar;
 	GtkWidget *statbar;
 	GtkWidget *window;
 
@@ -189,6 +218,10 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
 
 	gtk_box_pack_start(GTK_BOX(vbox), notebook, TRUE, TRUE, 0);
 
+	info_bar = perf_gtk__setup_info_bar();
+	if (info_bar)
+		gtk_box_pack_start(GTK_BOX(vbox), info_bar, FALSE, FALSE, 0);
+
 	statbar = perf_gtk__setup_statusbar();
 	gtk_box_pack_start(GTK_BOX(vbox), statbar, FALSE, FALSE, 0);
 
diff --git a/tools/perf/ui/gtk/gtk.h b/tools/perf/ui/gtk/gtk.h
index 2061678..a4d0f2b 100644
--- a/tools/perf/ui/gtk/gtk.h
+++ b/tools/perf/ui/gtk/gtk.h
@@ -10,6 +10,11 @@
 
 struct perf_gtk_context {
 	GtkWidget *main_window;
+
+#ifdef HAVE_GTK_INFO_BAR
+	GtkWidget *info_bar;
+	GtkWidget *message_label;
+#endif
 	GtkWidget *statbar;
 	guint statbar_ctx_id;
 };
@@ -24,4 +29,11 @@ static inline bool perf_gtk__is_active_context(struct perf_gtk_context *ctx)
 struct perf_gtk_context *perf_gtk__activate_context(GtkWidget *window);
 int perf_gtk__deactivate_context(struct perf_gtk_context **ctx);
 
+#ifndef HAVE_GTK_INFO_BAR
+static inline GtkWidget *perf_gtk__setup_info_bar(void)
+{
+	return NULL;
+}
+#endif
+
 #endif /* _PERF_GTK_H_ */

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [tip:perf/core] perf ui/gtk: Use struct perf_error_ops
  2012-05-29  4:23 ` [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops Namhyung Kim
@ 2012-06-20 16:49   ` tip-bot for Namhyung Kim
  0 siblings, 0 replies; 22+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-06-20 16:49 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, penberg,
	namhyung, tglx

Commit-ID:  e078ba14dff5c315ce19a978574883f417d2cfa0
Gitweb:     http://git.kernel.org/tip/e078ba14dff5c315ce19a978574883f417d2cfa0
Author:     Namhyung Kim <namhyung@gmail.com>
AuthorDate: Tue, 29 May 2012 13:23:02 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 19 Jun 2012 13:06:19 -0300

perf ui/gtk: Use struct perf_error_ops

Define and use perf_gtk_eops to provide a GTK2 message dialog for error
reporting and a info_bar for warning.

As GtkInfoBar requires recent GTK+ libraries, provides a fallback
implementation using statusbar widget too.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
Acked-by: Pekka Enberg <penberg@kernel.org>
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/1338265382-6872-8-git-send-email-namhyung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/gtk/setup.c |    5 +++
 tools/perf/ui/gtk/util.c  |   86 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 0 deletions(-)

diff --git a/tools/perf/ui/gtk/setup.c b/tools/perf/ui/gtk/setup.c
index 8295299..92879ce 100644
--- a/tools/perf/ui/gtk/setup.c
+++ b/tools/perf/ui/gtk/setup.c
@@ -1,12 +1,17 @@
 #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(&perf_gtk_eops);
 	return gtk_init_check(NULL, NULL) ? 0 : -1;
 }
 
 void perf_gtk__exit(bool wait_for_ok __used)
 {
+	perf_error__unregister(&perf_gtk_eops);
 	gtk_main_quit();
 }
diff --git a/tools/perf/ui/gtk/util.c b/tools/perf/ui/gtk/util.c
index 6fe13fd..0ead373 100644
--- a/tools/perf/ui/gtk/util.c
+++ b/tools/perf/ui/gtk/util.c
@@ -2,6 +2,8 @@
 #include "../../util/debug.h"
 #include "gtk.h"
 
+#include <string.h>
+
 
 struct perf_gtk_context *pgctx;
 
@@ -26,6 +28,90 @@ int perf_gtk__deactivate_context(struct perf_gtk_context **ctx)
 	return 0;
 }
 
+static int perf_gtk__error(const char *format, va_list args)
+{
+	char *msg;
+	GtkWidget *dialog;
+
+	if (!perf_gtk__is_active_context(pgctx) ||
+	    vasprintf(&msg, format, args) < 0) {
+		fprintf(stderr, "Error:\n");
+		vfprintf(stderr, format, args);
+		fprintf(stderr, "\n");
+		return -1;
+	}
+
+	dialog = gtk_message_dialog_new_with_markup(GTK_WINDOW(pgctx->main_window),
+					GTK_DIALOG_DESTROY_WITH_PARENT,
+					GTK_MESSAGE_ERROR,
+					GTK_BUTTONS_CLOSE,
+					"<b>Error</b>\n\n%s", msg);
+	gtk_dialog_run(GTK_DIALOG(dialog));
+
+	gtk_widget_destroy(dialog);
+	free(msg);
+	return 0;
+}
+
+#ifdef HAVE_GTK_INFO_BAR
+static int perf_gtk__warning_info_bar(const char *format, va_list args)
+{
+	char *msg;
+
+	if (!perf_gtk__is_active_context(pgctx) ||
+	    vasprintf(&msg, format, args) < 0) {
+		fprintf(stderr, "Warning:\n");
+		vfprintf(stderr, format, args);
+		fprintf(stderr, "\n");
+		return -1;
+	}
+
+	gtk_label_set_text(GTK_LABEL(pgctx->message_label), msg);
+	gtk_info_bar_set_message_type(GTK_INFO_BAR(pgctx->info_bar),
+				      GTK_MESSAGE_WARNING);
+	gtk_widget_show(pgctx->info_bar);
+
+	free(msg);
+	return 0;
+}
+#else
+static int perf_gtk__warning_statusbar(const char *format, va_list args)
+{
+	char *msg, *p;
+
+	if (!perf_gtk__is_active_context(pgctx) ||
+	    vasprintf(&msg, format, args) < 0) {
+		fprintf(stderr, "Warning:\n");
+		vfprintf(stderr, format, args);
+		fprintf(stderr, "\n");
+		return -1;
+	}
+
+	gtk_statusbar_pop(GTK_STATUSBAR(pgctx->statbar),
+			  pgctx->statbar_ctx_id);
+
+	/* Only first line can be displayed */
+	p = strchr(msg, '\n');
+	if (p)
+		*p = '\0';
+
+	gtk_statusbar_push(GTK_STATUSBAR(pgctx->statbar),
+			   pgctx->statbar_ctx_id, msg);
+
+	free(msg);
+	return 0;
+}
+#endif
+
+struct perf_error_ops perf_gtk_eops = {
+	.error		= perf_gtk__error,
+#ifdef HAVE_GTK_INFO_BAR
+	.warning	= perf_gtk__warning_info_bar,
+#else
+	.warning	= perf_gtk__warning_statusbar,
+#endif
+};
+
 /*
  * FIXME: Functions below should be implemented properly.
  *        For now, just add stubs for NO_NEWT=1 build.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops
  2012-05-07  8:26         ` Namhyung Kim
@ 2012-05-07  8:40           ` Ingo Molnar
  0 siblings, 0 replies; 22+ messages in thread
From: Ingo Molnar @ 2012-05-07  8:40 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Pekka Enberg, Arnaldo Carvalho de Melo, Peter Zijlstra,
	Paul Mackerras, Ingo Molnar, LKML


* 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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops
  2012-05-07  8:10       ` Ingo Molnar
@ 2012-05-07  8:26         ` Namhyung Kim
  2012-05-07  8:40           ` Ingo Molnar
  0 siblings, 1 reply; 22+ messages in thread
From: Namhyung Kim @ 2012-05-07  8:26 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Pekka Enberg, Arnaldo Carvalho de Melo, Peter Zijlstra,
	Paul Mackerras, Ingo Molnar, LKML

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops
  2012-04-30  8:31     ` Namhyung Kim
@ 2012-05-07  8:10       ` Ingo Molnar
  2012-05-07  8:26         ` Namhyung Kim
  0 siblings, 1 reply; 22+ messages in thread
From: Ingo Molnar @ 2012-05-07  8:10 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Pekka Enberg, Arnaldo Carvalho de Melo, Peter Zijlstra,
	Paul Mackerras, Ingo Molnar, LKML


* 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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops
  2012-04-30  6:31   ` Pekka Enberg
@ 2012-04-30  8:31     ` Namhyung Kim
  2012-05-07  8:10       ` Ingo Molnar
  0 siblings, 1 reply; 22+ messages in thread
From: Namhyung Kim @ 2012-04-30  8:31 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
	Ingo Molnar, LKML

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops
  2012-04-30  4:55 ` [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops Namhyung Kim
@ 2012-04-30  6:31   ` Pekka Enberg
  2012-04-30  8:31     ` Namhyung Kim
  0 siblings, 1 reply; 22+ messages in thread
From: Pekka Enberg @ 2012-04-30  6:31 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
	Ingo Molnar, Namhyung Kim, LKML

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops
  2012-04-30  4:55 [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end Namhyung Kim
@ 2012-04-30  4:55 ` Namhyung Kim
  2012-04-30  6:31   ` Pekka Enberg
  0 siblings, 1 reply; 22+ messages in thread
From: Namhyung Kim @ 2012-04-30  4:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML,
	Pekka Enberg

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2012-06-20 16:49 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-29  4:22 [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end (v3) Namhyung Kim
2012-05-29  4:22 ` [PATCH RESEND 1/7] perf ui: Make --stdio default when TUI is not supported Namhyung Kim
2012-05-29  4:22 ` [PATCH 2/7] perf tools: Convert critical messages to ui__error() Namhyung Kim
2012-05-30  7:49   ` [tip:perf/urgent] " tip-bot for Namhyung Kim
2012-05-29  4:22 ` [PATCH 3/7] perf ui: Introduce struct perf_error_ops Namhyung Kim
2012-06-20 16:45   ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-05-29  4:22 ` [PATCH 4/7] perf ui/gtk: Introduce struct perf_gtk_context Namhyung Kim
2012-06-20 16:46   ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-05-29  4:23 ` [PATCH 5/7] perf ui/gtk: Add GTK statusbar widget to browser window Namhyung Kim
2012-06-20 16:47   ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-05-29  4:23 ` [PATCH 6/7] perf ui/gtk: Add GTK info_bar " Namhyung Kim
2012-06-20 16:48   ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-05-29  4:23 ` [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops Namhyung Kim
2012-06-20 16:49   ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-05-30  1:31 ` [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end (v3) Arnaldo Carvalho de Melo
2012-05-30  6:19   ` Pekka Enberg
  -- strict thread matches above, loose matches on Subject: below --
2012-04-30  4:55 [PATCH 0/7] perf ui: Add basic error handling for GTK2 front-end Namhyung Kim
2012-04-30  4:55 ` [PATCH 7/7] perf ui/gtk: Use struct perf_error_ops Namhyung Kim
2012-04-30  6:31   ` Pekka Enberg
2012-04-30  8:31     ` Namhyung Kim
2012-05-07  8:10       ` Ingo Molnar
2012-05-07  8:26         ` Namhyung Kim
2012-05-07  8:40           ` Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®