[PATCH 1/3] trace-cmd: Fix tracecmd_recorder leaking file descriptors

[PATCH 1/3] trace-cmd: Fix tracecmd_recorder leaking file descriptors

[Slavomir Kaslev]

tracecmd_recorder is leaking brass pipe() file descriptors it creates for
splice()-ing. Fix this.

Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
 lib/trace-cmd/trace-recorder.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/trace-cmd/trace-recorder.c b/lib/trace-cmd/trace-recorder.c
index 5331925..b941cdc 100644
--- a/lib/trace-cmd/trace-recorder.c
+++ b/lib/trace-cmd/trace-recorder.c
@@ -88,6 +88,12 @@ void tracecmd_free_recorder(struct tracecmd_recorder *recorder)
 		append_file(recorder->page_size, recorder->fd1, recorder->fd2);
 	}
  close:
+	if (recorder->brass[0] >= 0)
+		close(recorder->brass[0]);
+
+	if (recorder->brass[1] >= 0)
+		close(recorder->brass[1]);
+
 	if (recorder->trace_fd >= 0)
 		close(recorder->trace_fd);
 
-- 
2.19.1

[PATCH 2/3] trace-cmd: Add defines for splice() flags on systems with old glibc

[Slavomir Kaslev]

Use SPLICE_F_ defines to refer to splice() flags instead of their numeric
values. This should be only necessary on systems with glibc version < 2.5.

No changes in behavior intended.

Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
 lib/trace-cmd/trace-recorder.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/lib/trace-cmd/trace-recorder.c b/lib/trace-cmd/trace-recorder.c
index b941cdc..1b6a995 100644
--- a/lib/trace-cmd/trace-recorder.c
+++ b/lib/trace-cmd/trace-recorder.c
@@ -26,6 +26,13 @@
 # define F_GETPIPE_SZ	1032 /* The Linux number for the option */
 #endif
 
+#ifndef SPLICE_F_MOVE
+# define SPLICE_F_MOVE		1
+# define SPLICE_F_NONBLOCK	2
+# define SPLICE_F_MORE		4
+# define SPLICE_F_GIFT		8
+#endif
+
 struct tracecmd_recorder {
 	int		fd;
 	int		fd1;
@@ -122,10 +129,10 @@ tracecmd_create_buffer_recorder_fd2(int fd, int fd2, int cpu, unsigned flags,
 	recorder->cpu = cpu;
 	recorder->flags = flags;
 
-	recorder->fd_flags = 1; /* SPLICE_F_MOVE */
+	recorder->fd_flags = SPLICE_F_MOVE;
 
 	if (!(recorder->flags & TRACECMD_RECORD_BLOCK))
-		recorder->fd_flags |= 2; /* and NON_BLOCK */
+		recorder->fd_flags |= SPLICE_F_NONBLOCK;
 
 	/* Init to know what to free and release */
 	recorder->trace_fd = -1;
@@ -349,7 +356,7 @@ static long splice_data(struct tracecmd_recorder *recorder)
 	long ret;
 
 	read = splice(recorder->trace_fd, NULL, recorder->brass[1], NULL,
-		      recorder->pipe_size, 1 /* SPLICE_F_MOVE */);
+		      recorder->pipe_size, SPLICE_F_MOVE);
 	if (read < 0) {
 		if (errno != EAGAIN && errno != EINTR) {
 			warning("recorder error in splice input");
@@ -421,7 +428,7 @@ static void set_nonblock(struct tracecmd_recorder *recorder)
 	fcntl(recorder->trace_fd, F_SETFL, flags | O_NONBLOCK);
 
 	/* Do not block on streams for write */
-	recorder->fd_flags |= 2; /* NON_BLOCK */
+	recorder->fd_flags |= SPLICE_F_NONBLOCK;
 }
 
 long tracecmd_flush_recording(struct tracecmd_recorder *recorder)
-- 
2.19.1

[PATCH 3/3] trace-cmd: Removed some unused #includes

[Slavomir Kaslev]

This patch removes several unused #include directives.

No changes in behaviour intended.

Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
 lib/trace-cmd/trace-input.c    | 9 ---------
 lib/trace-cmd/trace-recorder.c | 9 +--------
 2 files changed, 1 insertion(+), 17 deletions(-)

diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
index b628570..34a8301 100644
--- a/lib/trace-cmd/trace-input.c
+++ b/lib/trace-cmd/trace-input.c
@@ -4,22 +4,13 @@
  *
  */
 #define _LARGEFILE64_SOURCE
-#include <dirent.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <string.h>
-#include <getopt.h>
-#include <stdarg.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <sys/wait.h>
 #include <sys/mman.h>
-#include <pthread.h>
 #include <regex.h>
 #include <fcntl.h>
 #include <unistd.h>
-#include <ctype.h>
 #include <errno.h>
 
 #include <linux/time64.h>
diff --git a/lib/trace-cmd/trace-recorder.c b/lib/trace-cmd/trace-recorder.c
index 1b6a995..0b8d98a 100644
--- a/lib/trace-cmd/trace-recorder.c
+++ b/lib/trace-cmd/trace-recorder.c
@@ -4,18 +4,11 @@
  *
  */
 #define _LARGEFILE64_SOURCE
-#include <dirent.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <string.h>
-#include <stdarg.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <sys/wait.h>
-#include <pthread.h>
 #include <fcntl.h>
+#include <time.h>
 #include <unistd.h>
-#include <ctype.h>
 #include <errno.h>
 
 #include "trace-cmd.h"
-- 
2.19.1

Re: [PATCH 3/3] trace-cmd: Removed some unused #includes

[Steven Rostedt]

On Thu, 14 Feb 2019 16:07:23 +0200
Slavomir Kaslev <kaslevs@vmware.com> wrote:

> This patch removes several unused #include directives.
> 
> No changes in behaviour intended.
> 
> Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
> ---
>  lib/trace-cmd/trace-input.c    | 9 ---------
>  lib/trace-cmd/trace-recorder.c | 9 +--------
>  2 files changed, 1 insertion(+), 17 deletions(-)
> 
> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index b628570..34a8301 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -4,22 +4,13 @@
>   *
>   */
>  #define _LARGEFILE64_SOURCE
> -#include <dirent.h>
>  #include <stdbool.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> -#include <string.h>
> -#include <getopt.h>
> -#include <stdarg.h>
> -#include <sys/types.h>
> -#include <sys/stat.h>
> -#include <sys/wait.h>
>  #include <sys/mman.h>
> -#include <pthread.h>
>  #include <regex.h>
>  #include <fcntl.h>
>  #include <unistd.h>
> -#include <ctype.h>
>  #include <errno.h>
>  
>  #include <linux/time64.h>
> diff --git a/lib/trace-cmd/trace-recorder.c b/lib/trace-cmd/trace-recorder.c
> index 1b6a995..0b8d98a 100644
> --- a/lib/trace-cmd/trace-recorder.c
> +++ b/lib/trace-cmd/trace-recorder.c
> @@ -4,18 +4,11 @@
>   *
>   */
>  #define _LARGEFILE64_SOURCE
> -#include <dirent.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> -#include <string.h>
> -#include <stdarg.h>
> -#include <sys/types.h>
> -#include <sys/stat.h>
> -#include <sys/wait.h>
> -#include <pthread.h>
>  #include <fcntl.h>
> +#include <time.h>
>  #include <unistd.h>
> -#include <ctype.h>
>  #include <errno.h>
>  
>  #include "trace-cmd.h"

Nice cleanup.

Just an FYI, even on random patches, it's still good to have a cover
email for more than one patch series "[PATCH 0/3]", as it organizes it
better in inboxes.

BTW, I applied all of these.

Thanks Slavomir!

-- Steve