linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH trace-cmd 0/5] Miscellaneous fixes for trace-cmd
@ 2013-10-24 19:14 Seth Forshee
  2013-10-24 19:14 ` [PATCH trace-cmd 1/5] trace-cmd/listen: Remove use of sighandler_t Seth Forshee
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Seth Forshee @ 2013-10-24 19:14 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel, Seth Forshee

Hi Steve,

I've been working on getting trace-cmd packaged for Debian. In the
process I had to make several changes to either fix bugs or get the
tools to stop complaining. All of these look appropriate for upstream,
so I'm sending them your way.

Thanks,
Seth


Seth Forshee (5):
  trace-cmd/listen: Remove use of sighandler_t
  build: Install data files without execute permissions
  Add missing libgen.h includes
  Documentation: Add kernelshark.1.txt
  build: Use CFLAGS and LDFLAGS when building python bits

 Documentation/Makefile          | 13 ++++++------
 Documentation/kernelshark.1.txt | 46 +++++++++++++++++++++++++++++++++++++++++
 Makefile                        | 32 +++++++++++++++++++---------
 kernel-shark.c                  |  1 +
 trace-graph-main.c              |  1 +
 trace-listen.c                  |  2 +-
 trace-view-main.c               |  1 +
 7 files changed, 79 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/kernelshark.1.txt


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

* [PATCH trace-cmd 1/5] trace-cmd/listen: Remove use of sighandler_t
  2013-10-24 19:14 [PATCH trace-cmd 0/5] Miscellaneous fixes for trace-cmd Seth Forshee
@ 2013-10-24 19:14 ` Seth Forshee
  2013-10-24 20:30   ` Seth Forshee
  2013-10-24 19:14 ` [PATCH trace-cmd 2/5] build: Install data files without execute permissions Seth Forshee
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Seth Forshee @ 2013-10-24 19:14 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel, Seth Forshee

sighandler_t is a GNU extension and may not be defined if
_GNU_SOURCE is not set. To minimize the potential for build
problems, change signal_setup() to declare its handle argument
the same way struct sigaction declares the sa_handler member.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 trace-listen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/trace-listen.c b/trace-listen.c
index 8b8f02c..0019089 100644
--- a/trace-listen.c
+++ b/trace-listen.c
@@ -69,7 +69,7 @@ static void put_temp_file(char *file)
 
 #define MAX_PATH 1024
 
-static void signal_setup(int sig, sighandler_t handle)
+static void signal_setup(int sig, void (*handle)(int))
 {
 	struct sigaction action;
 
-- 
1.8.3.2


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

* [PATCH trace-cmd 2/5] build: Install data files without execute permissions
  2013-10-24 19:14 [PATCH trace-cmd 0/5] Miscellaneous fixes for trace-cmd Seth Forshee
  2013-10-24 19:14 ` [PATCH trace-cmd 1/5] trace-cmd/listen: Remove use of sighandler_t Seth Forshee
@ 2013-10-24 19:14 ` Seth Forshee
  2013-10-25  8:31   ` Steven Rostedt
  2013-10-24 19:14 ` [PATCH trace-cmd 3/5] Add missing libgen.h includes Seth Forshee
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Seth Forshee @ 2013-10-24 19:14 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel, Seth Forshee

Libraries and data files should not be installed with execute
permissions as is currently the case. Add do_data_install
functions to the makefiles to install data without execute
permissions and use this for installing all data files.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 Documentation/Makefile | 13 +++++++------
 Makefile               | 24 ++++++++++++++++++------
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index ad53c3b..21e42fd 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -73,21 +73,22 @@ all: $(MAN1) $(MAN5)
 
 # Need to find out how to export a macro instead of
 # copying this from the main Makefile.
-define do_install
+define do_install_data
 	$(print_install)				\
 	if [ ! -d '$(DESTDIR_SQ)$2' ]; then		\
 		$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2';	\
 	fi;						\
-	$(INSTALL) $1 '$(DESTDIR_SQ)$2'
+	$(INSTALL) -m 644 $1 '$(DESTDIR_SQ)$2'
 endef
 
+
 MAN1_INSTALL = $(MAN1:%.1=%.1.install)
 MAN5_INSTALL = $(MAN5:%.5=%.5.install)
 
 $(MAN1_INSTALL): %.1.install : %.1 force
-	$(Q)$(call do_install,$<,$(man_dir_SQ)/man1)
+	$(Q)$(call do_install_data,$<,$(man_dir_SQ)/man1)
 $(MAN5_INSTALL): %.5.install : %.5 force
-	$(Q)$(call do_install,$<,$(man_dir_SQ)/man5)
+	$(Q)$(call do_install_data,$<,$(man_dir_SQ)/man5)
 
 html_dir = $(src)/HTML
 image_dir = $(html_dir)/images
@@ -99,10 +100,10 @@ HTML_INSTALL = $(subst .html,.html.install,$(HTML))
 IMGS_INSTALL = $(subst .png,.png.install,$(IMGS))
 
 $(HTML_INSTALL): %.html.install : %.html force
-	$(Q)$(call do_install,$<,'$(html_install_SQ)')
+	$(Q)$(call do_install_data,$<,'$(html_install_SQ)')
 
 $(IMGS_INSTALL): %.png.install : %.png force
-	$(Q)$(call do_install,$<,'$(img_install_SQ)')
+	$(Q)$(call do_install_data,$<,'$(img_install_SQ)')
 
 
 GUI_INSTALL = $(HTML_INSTALL) $(IMGS_INSTALL)
diff --git a/Makefile b/Makefile
index 1964949..2423038 100644
--- a/Makefile
+++ b/Makefile
@@ -86,7 +86,8 @@ ifeq ($(shell sh -c "python-config --includes > /dev/null 2>&1 && echo y"), y)
 	PYTHON_PLUGINS := plugin_python.so
 	BUILD_PYTHON := $(PYTHON) $(PYTHON_PLUGINS)
 	PYTHON_SO_INSTALL := ctracecmd.install
-	PYTHON_PY_INSTALL := event-viewer.install tracecmd.install tracecmdgui.install
+	PYTHON_PY_PROGS := event-viewer.install
+	PYTHON_PY_LIBS := tracecmd.install tracecmdgui.install
 endif
 endif # NO_PYTHON
 
@@ -506,21 +507,32 @@ define do_install
 	$(INSTALL) $1 '$(DESTDIR_SQ)$2'
 endef
 
+define do_install_data
+	$(print_install)				\
+	if [ ! -d '$(DESTDIR_SQ)$2' ]; then		\
+		$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2';	\
+	fi;						\
+	$(INSTALL) -m 644 $1 '$(DESTDIR_SQ)$2'
+endef
+
 $(PLUGINS_INSTALL): %.install : %.so force
-	$(Q)$(call do_install,$<,$(plugin_dir_SQ))
+	$(Q)$(call do_install_data,$<,$(plugin_dir_SQ))
 
 install_plugins: $(PLUGINS_INSTALL)
 
 $(PYTHON_SO_INSTALL): %.install : %.so force
-	$(Q)$(call do_install,$<,$(python_dir_SQ))
+	$(Q)$(call do_install_data,$<,$(python_dir_SQ))
 
-$(PYTHON_PY_INSTALL): %.install : %.py force
+$(PYTHON_PY_PROGS): %.install : %.py force
 	$(Q)$(call do_install,$<,$(python_dir_SQ))
 
+$(PYTHON_PY_LIBS): %.install : %.py force
+	$(Q)$(call do_install_data,$<,$(python_dir_SQ))
+
 $(PYTHON_PY_PLUGINS): %.install : %.py force
-	$(Q)$(call do_install,$<,$(plugin_dir_SQ))
+	$(Q)$(call do_install_data,$<,$(plugin_dir_SQ))
 
-install_python: $(PYTHON_SO_INSTALL) $(PYTHON_PY_INSTALL) $(PYTHON_PY_PLUGINS)
+install_python: $(PYTHON_SO_INSTALL) $(PYTHON_PY_PROGS) $(PYTHON_PY_LIBS) $(PYTHON_PY_PLUGINS)
 
 install_cmd: all_cmd install_plugins install_python
 	$(Q)$(call do_install,trace-cmd,$(bindir_SQ))
-- 
1.8.3.2


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

* [PATCH trace-cmd 3/5] Add missing libgen.h includes
  2013-10-24 19:14 [PATCH trace-cmd 0/5] Miscellaneous fixes for trace-cmd Seth Forshee
  2013-10-24 19:14 ` [PATCH trace-cmd 1/5] trace-cmd/listen: Remove use of sighandler_t Seth Forshee
  2013-10-24 19:14 ` [PATCH trace-cmd 2/5] build: Install data files without execute permissions Seth Forshee
@ 2013-10-24 19:14 ` Seth Forshee
  2013-10-24 19:14 ` [PATCH trace-cmd 4/5] Documentation: Add kernelshark.1.txt Seth Forshee
  2013-10-24 19:14 ` [PATCH trace-cmd 5/5] build: Use CFLAGS and LDFLAGS when building python bits Seth Forshee
  4 siblings, 0 replies; 14+ messages in thread
From: Seth Forshee @ 2013-10-24 19:14 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel, Seth Forshee

This is needed for basname(). Without the include the compiler
may assume that basename() returns int, which leads to a segfault
in some cases.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 kernel-shark.c     | 1 +
 trace-graph-main.c | 1 +
 trace-view-main.c  | 1 +
 3 files changed, 3 insertions(+)

diff --git a/kernel-shark.c b/kernel-shark.c
index ee4796a..0201427 100644
--- a/kernel-shark.c
+++ b/kernel-shark.c
@@ -28,6 +28,7 @@
 #include <gtk/gtk.h>
 #include <errno.h>
 #include <getopt.h>
+#include <libgen.h>
 
 #include "trace-compat.h"
 #include "trace-capture.h"
diff --git a/trace-graph-main.c b/trace-graph-main.c
index 1bc10c1..3654295 100644
--- a/trace-graph-main.c
+++ b/trace-graph-main.c
@@ -23,6 +23,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#include <libgen.h>
 
 #include "trace-cmd.h"
 #include "trace-graph.h"
diff --git a/trace-view-main.c b/trace-view-main.c
index 73dfa8a..935b8c1 100644
--- a/trace-view-main.c
+++ b/trace-view-main.c
@@ -23,6 +23,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#include <libgen.h>
 
 #include "trace-cmd.h"
 #include "trace-view.h"
-- 
1.8.3.2


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

* [PATCH trace-cmd 4/5] Documentation: Add kernelshark.1.txt
  2013-10-24 19:14 [PATCH trace-cmd 0/5] Miscellaneous fixes for trace-cmd Seth Forshee
                   ` (2 preceding siblings ...)
  2013-10-24 19:14 ` [PATCH trace-cmd 3/5] Add missing libgen.h includes Seth Forshee
@ 2013-10-24 19:14 ` Seth Forshee
  2013-11-11  6:54   ` Rob Landley
  2013-10-24 19:14 ` [PATCH trace-cmd 5/5] build: Use CFLAGS and LDFLAGS when building python bits Seth Forshee
  4 siblings, 1 reply; 14+ messages in thread
From: Seth Forshee @ 2013-10-24 19:14 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel, Seth Forshee

Add a man page for kernelshark.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 Documentation/kernelshark.1.txt | 46 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/kernelshark.1.txt

diff --git a/Documentation/kernelshark.1.txt b/Documentation/kernelshark.1.txt
new file mode 100644
index 0000000..6a217dd
--- /dev/null
+++ b/Documentation/kernelshark.1.txt
@@ -0,0 +1,46 @@
+KERNELSHARK(1)
+==============
+
+NAME
+----
+kernelshark - graphical reader for trace-cmd(1) output
+
+SYNOPSIS
+--------
+*kernelshark* ['OPTIONS']
+
+DESCRIPTION
+-----------
+KernelShark is a front end reader of trace-cmd(1) output. It reads a 
+trace-cmd.dat(5) formatted file and produces a graph and list view of the
+data.
+
+OPTIONS
+-------
+*-h*::
+    Display the help text.
+
+*-v*::
+    Display the kernelshark version and exit.
+
+*-i* 'input-file'::
+    Read trace data from the file 'input-file'. By default input is read from
+    'trace.dat'.
+
+SEE ALSO
+--------
+trace-cmd(1)
+
+AUTHOR
+------
+Written by Steven Rostedt, <rostedt@goodmis.org>
+
+RESOURCES
+---------
+git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/trace-cmd.git
+
+COPYING
+-------
+Copyright \(C) 2010 Red Hat, Inc. Free use of this software is granted under
+the terms of the GNU Public License (GPL).
+
-- 
1.8.3.2


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

* [PATCH trace-cmd 5/5] build: Use CFLAGS and LDFLAGS when building python bits
  2013-10-24 19:14 [PATCH trace-cmd 0/5] Miscellaneous fixes for trace-cmd Seth Forshee
                   ` (3 preceding siblings ...)
  2013-10-24 19:14 ` [PATCH trace-cmd 4/5] Documentation: Add kernelshark.1.txt Seth Forshee
@ 2013-10-24 19:14 ` Seth Forshee
  4 siblings, 0 replies; 14+ messages in thread
From: Seth Forshee @ 2013-10-24 19:14 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel, Seth Forshee

Distro build systems expect all objects to be built with the
flags they supply, and may complain when they are not.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 2423038..1d613c4 100644
--- a/Makefile
+++ b/Makefile
@@ -569,13 +569,13 @@ PYGTK_CFLAGS = `pkg-config --cflags pygtk-2.0`
 
 ctracecmd.so: $(TCMD_LIB_OBJS) ctracecmd.i
 	swig -Wall -python -noproxy ctracecmd.i
-	$(CC) -fpic -c $(PYTHON_INCLUDES)  ctracecmd_wrap.c
-	$(CC) --shared $(TCMD_LIB_OBJS) ctracecmd_wrap.o -o ctracecmd.so
+	$(CC) -fpic -c $(CFLAGS) $(PYTHON_INCLUDES)  ctracecmd_wrap.c
+	$(CC) --shared $(TCMD_LIB_OBJS) $(LDFLAGS) ctracecmd_wrap.o -o ctracecmd.so
 
 ctracecmdgui.so: $(TRACE_VIEW_OBJS) $(LIB_FILE)
 	swig -Wall -python -noproxy ctracecmdgui.i
 	$(CC) -fpic -c  $(CFLAGS) $(INCLUDES) $(PYTHON_INCLUDES) $(PYGTK_CFLAGS) ctracecmdgui_wrap.c
-	$(CC) --shared $^ $(LIBS) $(CONFIG_LIBS) ctracecmdgui_wrap.o -o ctracecmdgui.so
+	$(CC) --shared $^ $(LDFLAGS) $(LIBS) $(CONFIG_LIBS) ctracecmdgui_wrap.o -o ctracecmdgui.so
 
 PHONY += python
 python: $(PYTHON)
@@ -594,7 +594,7 @@ do_compile_python_plugin_obj =			\
 
 do_python_plugin_build =			\
 	($(print_plugin_build)			\
-	$(CC) $< -shared $(PYTHON_LDFLAGS) -o $@)
+	$(CC) $< -shared $(LDFLAGS) $(PYTHON_LDFLAGS) -o $@)
 
 plugin_python.o: %.o : $(src)/%.c trace_python_dir
 	$(Q)$(do_compile_python_plugin_obj)
-- 
1.8.3.2


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

* Re: [PATCH trace-cmd 1/5] trace-cmd/listen: Remove use of sighandler_t
  2013-10-24 19:14 ` [PATCH trace-cmd 1/5] trace-cmd/listen: Remove use of sighandler_t Seth Forshee
@ 2013-10-24 20:30   ` Seth Forshee
  2013-10-28 15:45     ` Seth Forshee
  0 siblings, 1 reply; 14+ messages in thread
From: Seth Forshee @ 2013-10-24 20:30 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel

On Thu, Oct 24, 2013 at 02:14:30PM -0500, Seth Forshee wrote:
> sighandler_t is a GNU extension and may not be defined if
> _GNU_SOURCE is not set. To minimize the potential for build
> problems, change signal_setup() to declare its handle argument
> the same way struct sigaction declares the sa_handler member.

I just saw that you've already got a commit to define _GNU_SOURCE on
master. I was packaging v2.2.1 and didn't notice it, so you can ignore
this one.


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

* Re: [PATCH trace-cmd 2/5] build: Install data files without execute permissions
  2013-10-24 19:14 ` [PATCH trace-cmd 2/5] build: Install data files without execute permissions Seth Forshee
@ 2013-10-25  8:31   ` Steven Rostedt
  2013-10-25 13:05     ` Seth Forshee
  0 siblings, 1 reply; 14+ messages in thread
From: Steven Rostedt @ 2013-10-25  8:31 UTC (permalink / raw)
  To: Seth Forshee; +Cc: linux-kernel

On Thu, 2013-10-24 at 14:14 -0500, Seth Forshee wrote:
> diff --git a/Makefile b/Makefile
> index 1964949..2423038 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -86,7 +86,8 @@ ifeq ($(shell sh -c "python-config --includes > /dev/null 2>&1 && echo y"), y)
>  	PYTHON_PLUGINS := plugin_python.so
>  	BUILD_PYTHON := $(PYTHON) $(PYTHON_PLUGINS)
>  	PYTHON_SO_INSTALL := ctracecmd.install
> -	PYTHON_PY_INSTALL := event-viewer.install tracecmd.install tracecmdgui.install
> +	PYTHON_PY_PROGS := event-viewer.install
> +	PYTHON_PY_LIBS := tracecmd.install tracecmdgui.install
>  endif
>  endif # NO_PYTHON
>  
> @@ -506,21 +507,32 @@ define do_install
>  	$(INSTALL) $1 '$(DESTDIR_SQ)$2'
>  endef
>  
> +define do_install_data
> +	$(print_install)				\
> +	if [ ! -d '$(DESTDIR_SQ)$2' ]; then		\
> +		$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2';	\
> +	fi;						\
> +	$(INSTALL) -m 644 $1 '$(DESTDIR_SQ)$2'
> +endef
> +
>  $(PLUGINS_INSTALL): %.install : %.so force
> -	$(Q)$(call do_install,$<,$(plugin_dir_SQ))
> +	$(Q)$(call do_install_data,$<,$(plugin_dir_SQ))
>  
>  install_plugins: $(PLUGINS_INSTALL)
>  
>  $(PYTHON_SO_INSTALL): %.install : %.so force
> -	$(Q)$(call do_install,$<,$(python_dir_SQ))
> +	$(Q)$(call do_install_data,$<,$(python_dir_SQ))

Hmm, I never realized that shared libraries are not suppose to be
executable. At first I thought this was a mistake, but looking at other
shared libraries on my system, it seems a lot are not executable
(although many are, but those may also be made by people like myself who
thought there were suppose to be).

Thanks,

-- Steve

>  
> -$(PYTHON_PY_INSTALL): %.install : %.py force
> +$(PYTHON_PY_PROGS): %.install : %.py force
>  	$(Q)$(call do_install,$<,$(python_dir_SQ))
>  
> +$(PYTHON_PY_LIBS): %.install : %.py force
> +	$(Q)$(call do_install_data,$<,$(python_dir_SQ))
> +
>  $(PYTHON_PY_PLUGINS): %.install : %.py force
> -	$(Q)$(call do_install,$<,$(plugin_dir_SQ))
> +	$(Q)$(call do_install_data,$<,$(plugin_dir_SQ))
>  
> -install_python: $(PYTHON_SO_INSTALL) $(PYTHON_PY_INSTALL) $(PYTHON_PY_PLUGINS)
> +install_python: $(PYTHON_SO_INSTALL) $(PYTHON_PY_PROGS) $(PYTHON_PY_LIBS) $(PYTHON_PY_PLUGINS)
>  
>  install_cmd: all_cmd install_plugins install_python
>  	$(Q)$(call do_install,trace-cmd,$(bindir_SQ))



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

* Re: [PATCH trace-cmd 2/5] build: Install data files without execute permissions
  2013-10-25  8:31   ` Steven Rostedt
@ 2013-10-25 13:05     ` Seth Forshee
  2013-10-25 13:42       ` Steven Rostedt
  0 siblings, 1 reply; 14+ messages in thread
From: Seth Forshee @ 2013-10-25 13:05 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel

On Fri, Oct 25, 2013 at 04:31:33AM -0400, Steven Rostedt wrote:
> On Thu, 2013-10-24 at 14:14 -0500, Seth Forshee wrote:
> > diff --git a/Makefile b/Makefile
> > index 1964949..2423038 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -86,7 +86,8 @@ ifeq ($(shell sh -c "python-config --includes > /dev/null 2>&1 && echo y"), y)
> >  	PYTHON_PLUGINS := plugin_python.so
> >  	BUILD_PYTHON := $(PYTHON) $(PYTHON_PLUGINS)
> >  	PYTHON_SO_INSTALL := ctracecmd.install
> > -	PYTHON_PY_INSTALL := event-viewer.install tracecmd.install tracecmdgui.install
> > +	PYTHON_PY_PROGS := event-viewer.install
> > +	PYTHON_PY_LIBS := tracecmd.install tracecmdgui.install
> >  endif
> >  endif # NO_PYTHON
> >  
> > @@ -506,21 +507,32 @@ define do_install
> >  	$(INSTALL) $1 '$(DESTDIR_SQ)$2'
> >  endef
> >  
> > +define do_install_data
> > +	$(print_install)				\
> > +	if [ ! -d '$(DESTDIR_SQ)$2' ]; then		\
> > +		$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2';	\
> > +	fi;						\
> > +	$(INSTALL) -m 644 $1 '$(DESTDIR_SQ)$2'
> > +endef
> > +
> >  $(PLUGINS_INSTALL): %.install : %.so force
> > -	$(Q)$(call do_install,$<,$(plugin_dir_SQ))
> > +	$(Q)$(call do_install_data,$<,$(plugin_dir_SQ))
> >  
> >  install_plugins: $(PLUGINS_INSTALL)
> >  
> >  $(PYTHON_SO_INSTALL): %.install : %.so force
> > -	$(Q)$(call do_install,$<,$(python_dir_SQ))
> > +	$(Q)$(call do_install_data,$<,$(python_dir_SQ))
> 
> Hmm, I never realized that shared libraries are not suppose to be
> executable. At first I thought this was a mistake, but looking at other
> shared libraries on my system, it seems a lot are not executable
> (although many are, but those may also be made by people like myself who
> thought there were suppose to be).

I don't know that it's a hard and fast rule, but what guidance I found
from searching online was in favor of them not being executable. That
kind of makes sense, since they can't be executed directly (at least not
the ones that trace-cmd installs).

If you disagree, it won't hurt my feelings if you drop those changes, so
long as you keep the ones which makes sure things like .png files aren't
executable ;-)

Thanks,
Seth

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

* Re: [PATCH trace-cmd 2/5] build: Install data files without execute permissions
  2013-10-25 13:05     ` Seth Forshee
@ 2013-10-25 13:42       ` Steven Rostedt
  0 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2013-10-25 13:42 UTC (permalink / raw)
  To: Seth Forshee; +Cc: linux-kernel

On Fri, 2013-10-25 at 08:05 -0500, Seth Forshee wrote:

> 
> If you disagree, it won't hurt my feelings if you drop those changes, so
> long as you keep the ones which makes sure things like .png files aren't
> executable ;-)

I pretty much don't care. I'll pull in the patches as is. I also had
Johannes Berg sitting next to me when I was reading your patch that
touched the python part of the build, and since him and Darren Hart were
the ones to add that, I had him give me a verbal "Reviewed-by".

-- Steve



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

* Re: [PATCH trace-cmd 1/5] trace-cmd/listen: Remove use of sighandler_t
  2013-10-24 20:30   ` Seth Forshee
@ 2013-10-28 15:45     ` Seth Forshee
  2013-11-01 13:40       ` Steven Rostedt
  0 siblings, 1 reply; 14+ messages in thread
From: Seth Forshee @ 2013-10-28 15:45 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel

On Thu, Oct 24, 2013 at 03:30:07PM -0500, Seth Forshee wrote:
> On Thu, Oct 24, 2013 at 02:14:30PM -0500, Seth Forshee wrote:
> > sighandler_t is a GNU extension and may not be defined if
> > _GNU_SOURCE is not set. To minimize the potential for build
> > problems, change signal_setup() to declare its handle argument
> > the same way struct sigaction declares the sa_handler member.
> 
> I just saw that you've already got a commit to define _GNU_SOURCE on
> master. I was packaging v2.2.1 and didn't notice it, so you can ignore
> this one.

Turns out that fix doesn't work all the time. The following is also
required.


>From 20ec8510d0ca2ac789b1ca9cf7b0a6dc2d02f14c Mon Sep 17 00:00:00 2001
From: Seth Forshee <seth.forshee@canonical.com>
Date: Mon, 28 Oct 2013 08:15:59 -0500
Subject: [PATCH] Makefile: Ensure _GNU_SOURCE really is always defined

When CLFAGS is set with a command argument a normal variable
assignment is not sufficient to ensure that _GNU_SOURCE will be
defined. Since this really is critical for building, use an
override directive when adding it to CFLAGS to ensure that it
really does get defined in all cases.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 1d613c4..a2ba385 100644
--- a/Makefile
+++ b/Makefile
@@ -223,7 +223,7 @@ CFLAGS ?= -g -Wall
 LDFLAGS ?=
 
 # Required CFLAGS
-CFLAGS += -D_GNU_SOURCE
+override CFLAGS += -D_GNU_SOURCE
 
 ifndef NO_PTRACE
 ifneq ($(call try-cc,$(SOURCE_PTRACE),),y)
-- 
1.8.3.2


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

* Re: [PATCH trace-cmd 1/5] trace-cmd/listen: Remove use of sighandler_t
  2013-10-28 15:45     ` Seth Forshee
@ 2013-11-01 13:40       ` Steven Rostedt
  0 siblings, 0 replies; 14+ messages in thread
From: Steven Rostedt @ 2013-11-01 13:40 UTC (permalink / raw)
  To: Seth Forshee; +Cc: linux-kernel

On Mon, 28 Oct 2013 10:45:41 -0500
Seth Forshee <seth.forshee@canonical.com> wrote:

> 
> >From 20ec8510d0ca2ac789b1ca9cf7b0a6dc2d02f14c Mon Sep 17 00:00:00 2001
> From: Seth Forshee <seth.forshee@canonical.com>
> Date: Mon, 28 Oct 2013 08:15:59 -0500
> Subject: [PATCH] Makefile: Ensure _GNU_SOURCE really is always defined
> 
> When CLFAGS is set with a command argument a normal variable
> assignment is not sufficient to ensure that _GNU_SOURCE will be
> defined. Since this really is critical for building, use an
> override directive when adding it to CFLAGS to ensure that it
> really does get defined in all cases.

Can you send this as a separate patch please. New patches should not
be replies. You can reply to a patch with a patch, but please change
the subject when you do so.

Thanks,

-- Steve

> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 1d613c4..a2ba385 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -223,7 +223,7 @@ CFLAGS ?= -g -Wall
>  LDFLAGS ?=
>  
>  # Required CFLAGS
> -CFLAGS += -D_GNU_SOURCE
> +override CFLAGS += -D_GNU_SOURCE
>  
>  ifndef NO_PTRACE
>  ifneq ($(call try-cc,$(SOURCE_PTRACE),),y)


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

* Re: [PATCH trace-cmd 4/5] Documentation: Add kernelshark.1.txt
  2013-10-24 19:14 ` [PATCH trace-cmd 4/5] Documentation: Add kernelshark.1.txt Seth Forshee
@ 2013-11-11  6:54   ` Rob Landley
  2013-11-11 13:47     ` Seth Forshee
  0 siblings, 1 reply; 14+ messages in thread
From: Rob Landley @ 2013-11-11  6:54 UTC (permalink / raw)
  To: Seth Forshee; +Cc: Steven Rostedt, linux-kernel, Seth Forshee

On 10/24/2013 02:14:33 PM, Seth Forshee wrote:
> Add a man page for kernelshark.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
>  Documentation/kernelshark.1.txt | 46  
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/kernelshark.1.txt
> 
> diff --git a/Documentation/kernelshark.1.txt  
> b/Documentation/kernelshark.1.txt
> new file mode 100644
> index 0000000..6a217dd
> --- /dev/null
> +++ b/Documentation/kernelshark.1.txt
> @@ -0,0 +1,46 @@
> +KERNELSHARK(1)
> +==============
> +
> +NAME
> +----
> +kernelshark - graphical reader for trace-cmd(1) output
> +

We're throwing random man pages into Documentation now, at the top  
level with no subdirectory?

Please don't.

Rob

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

* Re: [PATCH trace-cmd 4/5] Documentation: Add kernelshark.1.txt
  2013-11-11  6:54   ` Rob Landley
@ 2013-11-11 13:47     ` Seth Forshee
  0 siblings, 0 replies; 14+ messages in thread
From: Seth Forshee @ 2013-11-11 13:47 UTC (permalink / raw)
  To: Rob Landley; +Cc: Steven Rostedt, linux-kernel

On Mon, Nov 11, 2013 at 12:54:53AM -0600, Rob Landley wrote:
> On 10/24/2013 02:14:33 PM, Seth Forshee wrote:
> >Add a man page for kernelshark.
> >
> >Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> >---
> > Documentation/kernelshark.1.txt | 46
> >+++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 46 insertions(+)
> > create mode 100644 Documentation/kernelshark.1.txt
> >
> >diff --git a/Documentation/kernelshark.1.txt
> >b/Documentation/kernelshark.1.txt
> >new file mode 100644
> >index 0000000..6a217dd
> >--- /dev/null
> >+++ b/Documentation/kernelshark.1.txt
> >@@ -0,0 +1,46 @@
> >+KERNELSHARK(1)
> >+==============
> >+
> >+NAME
> >+----
> >+kernelshark - graphical reader for trace-cmd(1) output
> >+
> 
> We're throwing random man pages into Documentation now, at the top
> level with no subdirectory?
> 
> Please don't.

You do realize this patch is for trace-cmd and not for the kernel,
right?

Seth

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

end of thread, other threads:[~2013-11-11 13:47 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-24 19:14 [PATCH trace-cmd 0/5] Miscellaneous fixes for trace-cmd Seth Forshee
2013-10-24 19:14 ` [PATCH trace-cmd 1/5] trace-cmd/listen: Remove use of sighandler_t Seth Forshee
2013-10-24 20:30   ` Seth Forshee
2013-10-28 15:45     ` Seth Forshee
2013-11-01 13:40       ` Steven Rostedt
2013-10-24 19:14 ` [PATCH trace-cmd 2/5] build: Install data files without execute permissions Seth Forshee
2013-10-25  8:31   ` Steven Rostedt
2013-10-25 13:05     ` Seth Forshee
2013-10-25 13:42       ` Steven Rostedt
2013-10-24 19:14 ` [PATCH trace-cmd 3/5] Add missing libgen.h includes Seth Forshee
2013-10-24 19:14 ` [PATCH trace-cmd 4/5] Documentation: Add kernelshark.1.txt Seth Forshee
2013-11-11  6:54   ` Rob Landley
2013-11-11 13:47     ` Seth Forshee
2013-10-24 19:14 ` [PATCH trace-cmd 5/5] build: Use CFLAGS and LDFLAGS when building python bits Seth Forshee

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).