This patch adds fixes to trace-cmd for return value checking, EINTR handling,
function prototypes, and data offsets to initial patch escape sequence fix [1].
Thanks much.
-Mike
v2->v3:
Fix attachments (aka learn how to use Mutt)
v1->v2:
Add five related bug fix patches
Michael Sartain (6):
Fix bad force_token escape sequence
Fix unsigned return values being error checked as negative
Handle EINTR signal interrupts for read, write, open calls
Fix read / write data offsets in read / write loops
Fix function prototypes for __vwarning, __vpr_stat, and __vdie
Fix cases where string literals were passed as string format args
event-parse.c | 2 +-
event-utils.h | 4 +-
parse-utils.c | 2 +
trace-capture.c | 12 ++--
trace-cmd-local.h | 2 +-
trace-dialog.c | 4 +-
trace-filter.c | 10 +--
trace-input.c | 187 +++++++++++++++++++++++-------------------------------
trace-local.h | 2 +-
trace-msg.c | 13 ++--
10 files changed, 104 insertions(+), 134 deletions(-)
[1] https://www.mail-archive.com/[email protected]/msg1414382.html
--
2.11.0
Read/write/open calls weren't handling EINTR in trace-input.c
This patch uses the standard GNU C TEMP_FAILURE_RETRY macro to handle EINTR
return values, and updates read/write calls in trace-msg.c to match.
Signed-off-by: Michael Sartain <[email protected]>
---
trace-cmd-local.h | 2 +-
trace-input.c | 8 ++++----
trace-msg.c | 8 ++------
3 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/trace-cmd-local.h b/trace-cmd-local.h
index 9412f9d..8595a8a 100644
--- a/trace-cmd-local.h
+++ b/trace-cmd-local.h
@@ -31,7 +31,7 @@ static ssize_t __do_write(int fd, const void *data, size_t size)
ssize_t w;
do {
- w = write(fd, data, size - tot);
+ w = TEMP_FAILURE_RETRY(write(fd, data, size - tot));
tot += w;
if (!w)
diff --git a/trace-input.c b/trace-input.c
index 89ddcf5..251d32b 100644
--- a/trace-input.c
+++ b/trace-input.c
@@ -202,7 +202,7 @@ static ssize_t do_read(struct tracecmd_input *handle, void *data, size_t size)
ssize_t r;
do {
- r = read(handle->fd, data, size - tot);
+ r = TEMP_FAILURE_RETRY(read(handle->fd, data, size - tot));
tot += r;
if (!r)
@@ -774,7 +774,7 @@ static int read_page(struct tracecmd_input *handle, off64_t offset,
off64_t ret;
if (handle->use_pipe) {
- ret = read(handle->cpu_data[cpu].pipe_fd, map, handle->page_size);
+ ret = TEMP_FAILURE_RETRY(read(handle->cpu_data[cpu].pipe_fd, map, handle->page_size));
/* Set EAGAIN if the pipe is empty */
if (ret < 0) {
errno = EAGAIN;
@@ -2645,7 +2645,7 @@ struct tracecmd_input *tracecmd_alloc(const char *file)
{
int fd;
- fd = open(file, O_RDONLY);
+ fd = TEMP_FAILURE_RETRY(open(file, O_RDONLY));
if (fd < 0)
return NULL;
@@ -2686,7 +2686,7 @@ struct tracecmd_input *tracecmd_open(const char *file)
{
int fd;
- fd = open(file, O_RDONLY);
+ fd = TEMP_FAILURE_RETRY(open(file, O_RDONLY));
if (fd < 0)
return NULL;
diff --git a/trace-msg.c b/trace-msg.c
index 3991985..d358318 100644
--- a/trace-msg.c
+++ b/trace-msg.c
@@ -291,10 +291,8 @@ static int msg_read(int fd, void *buf, u32 size, int *n)
ssize_t r;
while (size) {
- r = read(fd, buf + *n, size);
+ r = TEMP_FAILURE_RETRY(read(fd, buf + *n, size));
if (r < 0) {
- if (errno == EINTR)
- continue;
return -errno;
} else if (!r)
return -ENOTCONN;
@@ -662,10 +660,8 @@ int tracecmd_msg_collect_metadata(int ifd, int ofd)
t = n;
s = 0;
do {
- s = write(ofd, msg.data.meta.buf+s, t);
+ s = TEMP_FAILURE_RETRY(write(ofd, msg.data.meta.buf+s, t));
if (s < 0) {
- if (errno == EINTR)
- continue;
warning("writing to file");
return -errno;
}
--
2.11.0
The tot variable in __do_write and do_read is incremented with the amount read
/ written, but subsequent times through the loop the calls continue to use the
original data pointer.
Signed-off-by: Michael Sartain <[email protected]>
---
trace-cmd-local.h | 2 +-
trace-input.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/trace-cmd-local.h b/trace-cmd-local.h
index 8595a8a..b8ab35b 100644
--- a/trace-cmd-local.h
+++ b/trace-cmd-local.h
@@ -31,7 +31,7 @@ static ssize_t __do_write(int fd, const void *data, size_t size)
ssize_t w;
do {
- w = TEMP_FAILURE_RETRY(write(fd, data, size - tot));
+ w = TEMP_FAILURE_RETRY(write(fd, data + tot, size - tot));
tot += w;
if (!w)
diff --git a/trace-input.c b/trace-input.c
index 251d32b..8395917 100644
--- a/trace-input.c
+++ b/trace-input.c
@@ -202,7 +202,7 @@ static ssize_t do_read(struct tracecmd_input *handle, void *data, size_t size)
ssize_t r;
do {
- r = TEMP_FAILURE_RETRY(read(handle->fd, data, size - tot));
+ r = TEMP_FAILURE_RETRY(read(handle->fd, data + tot, size - tot));
tot += r;
if (!r)
--
2.11.0
They were declared:
(const char *fmt, ...)
but implemented as:
(const char *fmt, va_list ap)
Signed-off-by: Michael Sartain <[email protected]>
---
event-utils.h | 4 ++--
parse-utils.c | 2 ++
trace-local.h | 2 +-
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/event-utils.h b/event-utils.h
index d1dc217..f709db2 100644
--- a/event-utils.h
+++ b/event-utils.h
@@ -31,8 +31,8 @@ void vpr_stat(const char *fmt, va_list ap);
void __warning(const char *fmt, ...);
void __pr_stat(const char *fmt, ...);
-void __vwarning(const char *fmt, ...);
-void __vpr_stat(const char *fmt, ...);
+void __vwarning(const char *fmt, va_list ap);
+void __vpr_stat(const char *fmt, va_list ap);
#define min(x, y) ({ \
typeof(x) _min1 = (x); \
diff --git a/parse-utils.c b/parse-utils.c
index 42c1885..5cc9688 100644
--- a/parse-utils.c
+++ b/parse-utils.c
@@ -23,6 +23,8 @@
#include <stdarg.h>
#include <errno.h>
+#include "event-utils.h"
+
#define __weak __attribute__((weak))
void __vwarning(const char *fmt, va_list ap)
diff --git a/trace-local.h b/trace-local.h
index fa987bc..fb9f599 100644
--- a/trace-local.h
+++ b/trace-local.h
@@ -198,6 +198,6 @@ int count_cpus(void);
void die(const char *fmt, ...); /* Can be overriden */
void *malloc_or_die(unsigned int size); /* Can be overridden */
void __die(const char *fmt, ...);
-void __vdie(const char *fmt, ...);
+void __vdie(const char *fmt, va_list ap);
#endif /* __TRACE_LOCAL_H */
--
2.11.0
Functions like read4 and read8 had unsigned return types but callers were
checking for those return values being less than 0 for errors.
This patch changes those functions to return signed int error values and take a
pointer to a size parameter. Also changes several locals to match the function
types.
Signed-off-by: Michael Sartain <[email protected]>
---
trace-input.c | 179 ++++++++++++++++++++++++----------------------------------
trace-msg.c | 5 +-
2 files changed, 78 insertions(+), 106 deletions(-)
diff --git a/trace-input.c b/trace-input.c
index e676c85..89ddcf5 100644
--- a/trace-input.c
+++ b/trace-input.c
@@ -196,10 +196,10 @@ static const char *show_records(struct list_head *pages)
static int init_cpu(struct tracecmd_input *handle, int cpu);
-static int do_read(struct tracecmd_input *handle, void *data, int size)
+static ssize_t do_read(struct tracecmd_input *handle, void *data, size_t size)
{
- int tot = 0;
- int r;
+ ssize_t tot = 0;
+ ssize_t r;
do {
r = read(handle->fd, data, size - tot);
@@ -214,10 +214,10 @@ static int do_read(struct tracecmd_input *handle, void *data, int size)
return tot;
}
-static int
-do_read_check(struct tracecmd_input *handle, void *data, int size)
+static ssize_t
+do_read_check(struct tracecmd_input *handle, void *data, size_t size)
{
- int ret;
+ ssize_t ret;
ret = do_read(handle, data, size);
if (ret < 0)
@@ -232,9 +232,9 @@ static char *read_string(struct tracecmd_input *handle)
{
char buf[BUFSIZ];
char *str = NULL;
- int size = 0;
- int i;
- int r;
+ size_t size = 0;
+ ssize_t i;
+ ssize_t r;
for (;;) {
r = do_read(handle, buf, BUFSIZ);
@@ -294,7 +294,7 @@ static char *read_string(struct tracecmd_input *handle)
return NULL;
}
-static unsigned int read4(struct tracecmd_input *handle)
+static int read4(struct tracecmd_input *handle, unsigned int *size)
{
struct pevent *pevent = handle->pevent;
unsigned int data;
@@ -302,10 +302,11 @@ static unsigned int read4(struct tracecmd_input *handle)
if (do_read_check(handle, &data, 4))
return -1;
- return __data2host4(pevent, data);
+ *size = __data2host4(pevent, data);
+ return 0;
}
-static unsigned long long read8(struct tracecmd_input *handle)
+static int read8(struct tracecmd_input *handle, unsigned long long *size)
{
struct pevent *pevent = handle->pevent;
unsigned long long data;
@@ -313,13 +314,14 @@ static unsigned long long read8(struct tracecmd_input *handle)
if (do_read_check(handle, &data, 8))
return -1;
- return __data2host8(pevent, data);
+ *size = __data2host8(pevent, data);
+ return 0;
}
static int read_header_files(struct tracecmd_input *handle)
{
struct pevent *pevent = handle->pevent;
- long long size;
+ unsigned long long size;
char *header;
char buf[BUFSIZ];
@@ -329,8 +331,7 @@ static int read_header_files(struct tracecmd_input *handle)
if (memcmp(buf, "header_page", 12) != 0)
return -1;
- size = read8(handle);
- if (size < 0)
+ if (read8(handle, &size) < 0)
return -1;
header = malloc(size);
@@ -355,8 +356,7 @@ static int read_header_files(struct tracecmd_input *handle)
if (memcmp(buf, "header_event", 13) != 0)
return -1;
- size = read8(handle);
- if (size < 0)
+ if (read8(handle, &size) < 0)
return -1;
header = malloc(size);
@@ -521,11 +521,10 @@ static int read_ftrace_files(struct tracecmd_input *handle, const char *regex)
regex_t epreg;
regex_t *sreg = NULL;
regex_t *ereg = NULL;
+ unsigned int count, i;
int print_all = 0;
int unique;
- int count;
int ret;
- int i;
if (regex) {
sreg = &spreg;
@@ -554,13 +553,11 @@ static int read_ftrace_files(struct tracecmd_input *handle, const char *regex)
}
}
- count = read4(handle);
- if (count < 0)
+ if (read4(handle, &count) < 0)
return -1;
for (i = 0; i < count; i++) {
- size = read8(handle);
- if (size < 0)
+ if (read8(handle, &size) < 0)
return -1;
ret = read_ftrace_file(handle, size, print_all, ereg);
if (ret < 0)
@@ -587,13 +584,13 @@ static int read_event_files(struct tracecmd_input *handle, const char *regex)
regex_t *sreg = NULL;
regex_t *ereg = NULL;
regex_t *reg;
- int systems;
+ unsigned int systems;
+ unsigned int count;
+ unsigned int i, x;
int print_all;
int sys_printed;
- int count;
int unique;
int ret;
- int i,x;
if (regex) {
sreg = &spreg;
@@ -603,8 +600,7 @@ static int read_event_files(struct tracecmd_input *handle, const char *regex)
return -1;
}
- systems = read4(handle);
- if (systems < 0)
+ if (read4(handle, &systems) < 0)
return -1;
for (i = 0; i < systems; i++) {
@@ -637,13 +633,11 @@ static int read_event_files(struct tracecmd_input *handle, const char *regex)
}
}
- count = read4(handle);
- if (count < 0)
+ if (read4(handle, &count) < 0)
goto failed;
for (x=0; x < count; x++) {
- size = read8(handle);
- if (size < 0)
+ if (read8(handle, &size) < 0)
goto failed;
ret = read_event_file(handle, system, size,
@@ -675,16 +669,14 @@ static int read_event_files(struct tracecmd_input *handle, const char *regex)
static int read_proc_kallsyms(struct tracecmd_input *handle)
{
struct pevent *pevent = handle->pevent;
- int size;
+ unsigned int size;
char *buf;
- size = read4(handle);
+ if (read4(handle, &size) < 0)
+ return -1;
if (!size)
return 0; /* OK? */
- if (size < 0)
- return -1;
-
buf = malloc(size+1);
if (!buf)
return -1;
@@ -702,16 +694,14 @@ static int read_proc_kallsyms(struct tracecmd_input *handle)
static int read_ftrace_printk(struct tracecmd_input *handle)
{
- int size;
+ unsigned int size;
char *buf;
- size = read4(handle);
+ if (read4(handle, &size) < 0)
+ return -1;
if (!size)
return 0; /* OK? */
- if (size < 0)
- return -1;
-
buf = malloc(size + 1);
if (!buf)
return -1;
@@ -2268,8 +2258,8 @@ static int read_cpu_data(struct tracecmd_input *handle)
if (pevent->old_format)
kbuffer_set_old_format(handle->cpu_data[cpu].kbuf);
- offset = read8(handle);
- size = read8(handle);
+ read8(handle, &offset);
+ read8(handle, &size);
handle->cpu_data[cpu].file_offset = offset;
handle->cpu_data[cpu].file_size = size;
@@ -2315,8 +2305,7 @@ static int read_cpu_data(struct tracecmd_input *handle)
static int read_data_and_size(struct tracecmd_input *handle,
char **data, unsigned long long *size)
{
- *size = read8(handle);
- if (*size < 0)
+ if (read8(handle, size) < 0)
return -1;
*data = malloc(*size + 1);
if (!*data)
@@ -2367,11 +2356,12 @@ static int read_and_parse_trace_clock(struct tracecmd_input *handle,
int tracecmd_init_data(struct tracecmd_input *handle)
{
struct pevent *pevent = handle->pevent;
+ unsigned int cpus;
int ret;
- handle->cpus = read4(handle);
- if (handle->cpus < 0)
+ if (read4(handle, &cpus) < 0)
return -1;
+ handle->cpus = cpus;
pevent_set_cpus(pevent, handle->cpus);
@@ -2570,6 +2560,7 @@ struct tracecmd_input *tracecmd_alloc_fd(int fd)
{
struct tracecmd_input *handle;
char test[] = { 23, 8, 68 };
+ unsigned int page_size;
char *version;
char buf[BUFSIZ];
@@ -2616,7 +2607,8 @@ struct tracecmd_input *tracecmd_alloc_fd(int fd)
do_read_check(handle, buf, 1);
handle->long_size = buf[0];
- handle->page_size = read4(handle);
+ read4(handle, &page_size);
+ handle->page_size = page_size;
handle->header_files_start =
lseek64(handle->fd, 0, SEEK_CUR);
@@ -2772,36 +2764,30 @@ void tracecmd_close(struct tracecmd_input *handle)
free(handle);
}
-static long long read_copy_size8(struct tracecmd_input *handle, int fd)
+static int read_copy_size8(struct tracecmd_input *handle, int fd, unsigned long long *size)
{
- long long size;
-
/* read size */
- if (do_read_check(handle, &size, 8))
+ if (do_read_check(handle, size, 8))
return -1;
- if (__do_write_check(fd, &size, 8))
+ if (__do_write_check(fd, size, 8))
return -1;
- size = __data2host8(handle->pevent, size);
-
- return size;
+ *size = __data2host8(handle->pevent, *size);
+ return 0;
}
-static int read_copy_size4(struct tracecmd_input *handle, int fd)
+static int read_copy_size4(struct tracecmd_input *handle, int fd, unsigned int *size)
{
- int size;
-
/* read size */
- if (do_read_check(handle, &size, 4))
+ if (do_read_check(handle, size, 4))
return -1;
- if (__do_write_check(fd, &size, 4))
+ if (__do_write_check(fd, size, 4))
return -1;
- size = __data2host4(handle->pevent, size);
-
- return size;
+ *size = __data2host4(handle->pevent, *size);
+ return 0;
}
static int read_copy_data(struct tracecmd_input *handle,
@@ -2829,7 +2815,7 @@ static int read_copy_data(struct tracecmd_input *handle,
static int copy_header_files(struct tracecmd_input *handle, int fd)
{
- long long size;
+ unsigned long long size;
lseek64(handle->fd, handle->header_files_start, SEEK_SET);
@@ -2837,8 +2823,7 @@ static int copy_header_files(struct tracecmd_input *handle, int fd)
if (read_copy_data(handle, 12, fd) < 0)
return -1;
- size = read_copy_size8(handle, fd);
- if (size < 0)
+ if (read_copy_size8(handle, fd, &size) < 0)
return -1;
if (read_copy_data(handle, size, fd) < 0)
@@ -2848,8 +2833,7 @@ static int copy_header_files(struct tracecmd_input *handle, int fd)
if (read_copy_data(handle, 13, fd) < 0)
return -1;
- size = read_copy_size8(handle, fd);
- if (size < 0)
+ if (read_copy_size8(handle, fd, &size) < 0)
return -1;
if (read_copy_data(handle, size, fd) < 0)
@@ -2861,17 +2845,15 @@ static int copy_header_files(struct tracecmd_input *handle, int fd)
static int copy_ftrace_files(struct tracecmd_input *handle, int fd)
{
unsigned long long size;
- int count;
- int i;
+ unsigned int count;
+ unsigned int i;
- count = read_copy_size4(handle, fd);
- if (count < 0)
+ if (read_copy_size4(handle, fd, &count) < 0)
return -1;
for (i = 0; i < count; i++) {
- size = read_copy_size8(handle, fd);
- if (size < 0)
+ if (read_copy_size8(handle, fd, &size) < 0)
return -1;
if (read_copy_data(handle, size, fd) < 0)
@@ -2885,13 +2867,11 @@ static int copy_event_files(struct tracecmd_input *handle, int fd)
{
unsigned long long size;
char *system;
- int systems;
- int count;
- int ret;
- int i,x;
+ unsigned int systems;
+ unsigned int count;
+ unsigned int i,x;
- systems = read_copy_size4(handle, fd);
- if (systems < 0)
+ if (read_copy_size4(handle, fd, &systems) < 0)
return -1;
for (i = 0; i < systems; i++) {
@@ -2904,17 +2884,14 @@ static int copy_event_files(struct tracecmd_input *handle, int fd)
}
free(system);
- count = read_copy_size4(handle, fd);
- if (count < 0)
+ if (read_copy_size4(handle, fd, &count) < 0)
return -1;
for (x=0; x < count; x++) {
- size = read_copy_size8(handle, fd);
- if (size < 0)
+ if (read_copy_size8(handle, fd, &size) < 0)
return -1;
- ret = read_copy_data(handle, size, fd);
- if (ret < 0)
+ if (read_copy_data(handle, size, fd) < 0)
return -1;
}
}
@@ -2924,15 +2901,13 @@ static int copy_event_files(struct tracecmd_input *handle, int fd)
static int copy_proc_kallsyms(struct tracecmd_input *handle, int fd)
{
- int size;
+ unsigned int size;
- size = read_copy_size4(handle, fd);
+ if (read_copy_size4(handle, fd, &size) < 0)
+ return -1;
if (!size)
return 0; /* OK? */
- if (size < 0)
- return -1;
-
if (read_copy_data(handle, size, fd) < 0)
return -1;
@@ -2941,15 +2916,13 @@ static int copy_proc_kallsyms(struct tracecmd_input *handle, int fd)
static int copy_ftrace_printk(struct tracecmd_input *handle, int fd)
{
- int size;
+ unsigned int size;
- size = read_copy_size4(handle, fd);
+ if (read_copy_size4(handle, fd, &size) < 0)
+ return -1;
if (!size)
return 0; /* OK? */
- if (size < 0)
- return -1;
-
if (read_copy_data(handle, size, fd) < 0)
return -1;
@@ -2958,15 +2931,13 @@ static int copy_ftrace_printk(struct tracecmd_input *handle, int fd)
static int copy_command_lines(struct tracecmd_input *handle, int fd)
{
- unsigned long size;
+ unsigned long long size;
- size = read_copy_size8(handle, fd);
+ if (read_copy_size8(handle, fd, &size) < 0)
+ return -1;
if (!size)
return 0; /* OK? */
- if (size < 0)
- return -1;
-
if (read_copy_data(handle, size, fd) < 0)
return -1;
diff --git a/trace-msg.c b/trace-msg.c
index f1b6546..3991985 100644
--- a/trace-msg.c
+++ b/trace-msg.c
@@ -288,7 +288,7 @@ static int tracecmd_msg_send(int fd, u32 cmd)
static int msg_read(int fd, void *buf, u32 size, int *n)
{
- int r;
+ ssize_t r;
while (size) {
r = read(fd, buf + *n, size);
@@ -637,7 +637,8 @@ int tracecmd_msg_finish_sending_metadata(int fd)
int tracecmd_msg_collect_metadata(int ifd, int ofd)
{
struct tracecmd_msg msg;
- u32 s, t, n, cmd;
+ u32 t, n, cmd;
+ ssize_t s;
int ret;
do {
--
2.11.0
Signed-off-by: Michael Sartain <[email protected]>
---
event-parse.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/event-parse.c b/event-parse.c
index 3217131..61f48c1 100644
--- a/event-parse.c
+++ b/event-parse.c
@@ -1093,7 +1093,7 @@ static enum event_type __read_token(char **tok)
if (strcmp(*tok, "LOCAL_PR_FMT") == 0) {
free(*tok);
*tok = NULL;
- return force_token("\"\%s\" ", tok);
+ return force_token("\"%s\" ", tok);
} else if (strcmp(*tok, "STA_PR_FMT") == 0) {
free(*tok);
*tok = NULL;
--
2.11.0
Signed-off-by: Michael Sartain <[email protected]>
---
trace-capture.c | 12 ++++++------
trace-dialog.c | 4 ++--
trace-filter.c | 10 +++++-----
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/trace-capture.c b/trace-capture.c
index 1acfe44..4e1392e 100644
--- a/trace-capture.c
+++ b/trace-capture.c
@@ -1019,7 +1019,7 @@ static void save_events(struct trace_capture *cap,
tracecmd_xml_write_element(handle, "CaptureType", "Events");
for (i = 0; systems && systems[i]; i++)
- tracecmd_xml_write_element(handle, "System", systems[i]);
+ tracecmd_xml_write_element(handle, "System", "%s", systems[i]);
if (!events || events[0] < 0)
return;
@@ -1029,8 +1029,8 @@ static void save_events(struct trace_capture *cap,
event = pevent_find_event(pevent, events[i]);
if (event) {
tracecmd_xml_start_sub_system(handle, "Event");
- tracecmd_xml_write_element(handle, "System", event->system);
- tracecmd_xml_write_element(handle, "Name", event->name);
+ tracecmd_xml_write_element(handle, "System", "%s", event->system);
+ tracecmd_xml_write_element(handle, "Name", "%s", event->name);
tracecmd_xml_end_sub_system(handle);
}
}
@@ -1068,15 +1068,15 @@ static void save_settings(struct trace_capture *cap, const char *filename)
update_plugin(cap);
if (info->cap_plugin)
- tracecmd_xml_write_element(handle, "Plugin", info->cap_plugin);
+ tracecmd_xml_write_element(handle, "Plugin", "%s", info->cap_plugin);
command = gtk_entry_get_text(GTK_ENTRY(cap->command_entry));
if (command && strlen(command) && !is_just_ws(command))
- tracecmd_xml_write_element(handle, "Command", command);
+ tracecmd_xml_write_element(handle, "Command", "%s", command);
file = gtk_entry_get_text(GTK_ENTRY(cap->file_entry));
if (file && strlen(file) && !is_just_ws(file))
- tracecmd_xml_write_element(handle, "File", file);
+ tracecmd_xml_write_element(handle, "File", "%s", file);
tracecmd_xml_end_system(handle);
diff --git a/trace-dialog.c b/trace-dialog.c
index 5d3aabe..b5776cc 100644
--- a/trace-dialog.c
+++ b/trace-dialog.c
@@ -218,7 +218,7 @@ void warning(const char *fmt, ...)
errno = 0;
trace_dialog(GTK_WINDOW(parent_window), TRACE_GUI_WARNING,
- str->str);
+ "%s", str->str);
g_string_free(str, TRUE);
}
@@ -425,7 +425,7 @@ void trace_show_record_dialog(GtkWindow *parent, struct pevent *pevent,
if (s.buffer_size) {
trace_seq_terminate(&s);
- trace_dialog(parent, TRACE_GUI_OTHER, s.buffer);
+ trace_dialog(parent, TRACE_GUI_OTHER, "%s", s.buffer);
}
trace_seq_destroy(&s);
diff --git a/trace-filter.c b/trace-filter.c
index 1c36d84..bd8cb11 100644
--- a/trace-filter.c
+++ b/trace-filter.c
@@ -2043,7 +2043,7 @@ int trace_filter_save_events(struct tracecmd_xml_handle *handle,
&event_ids);
for (i = 0; systems && systems[i]; i++)
- tracecmd_xml_write_element(handle, "System", systems[i]);
+ tracecmd_xml_write_element(handle, "System", "%s", systems[i]);
for (i = 0; event_ids && event_ids[i] > 0; i++) {
str = pevent_filter_make_string(filter, event_ids[i]);
@@ -2060,11 +2060,11 @@ int trace_filter_save_events(struct tracecmd_xml_handle *handle,
}
tracecmd_xml_start_sub_system(handle, "Event");
- tracecmd_xml_write_element(handle, "System", event->system);
- tracecmd_xml_write_element(handle, "Name", event->name);
+ tracecmd_xml_write_element(handle, "System", "%s", event->system);
+ tracecmd_xml_write_element(handle, "Name", "%s", event->name);
/* If this is has an advanced filter, include that too */
if (strcmp(str, "TRUE") != 0) {
- tracecmd_xml_write_element(handle, "Advanced",
+ tracecmd_xml_write_element(handle, "Advanced", "%s",
str);
}
tracecmd_xml_end_sub_system(handle);
@@ -2088,7 +2088,7 @@ int trace_filter_save_tasks(struct tracecmd_xml_handle *handle,
for (i = 0; pids[i] >= 0; i++) {
snprintf(buffer, 100, "%d", pids[i]);
- tracecmd_xml_write_element(handle, "Task", buffer);
+ tracecmd_xml_write_element(handle, "Task", "%s", buffer);
}
free(pids);
--
2.11.0
On Wed, 14 Jun 2017 18:27:58 -0600
Michael Sartain <[email protected]> wrote:
> Read/write/open calls weren't handling EINTR in trace-input.c
>
> This patch uses the standard GNU C TEMP_FAILURE_RETRY macro to handle EINTR
> return values, and updates read/write calls in trace-msg.c to match.
I understand that GNU C gives the TEMP_FAILURE_RETRY() macro, but I
find it really ugly, and takes away from the flow of the code. Perhaps
we should just add our own static inline functions of the same name:
write_intr(), read_intr(), open_intr()
that does the same thing and use that instead. The inline functions
could even use the TEMP_FAILURE_RETRY(). I just don't want that ugly
name scattered in the .c code.
Thanks!
-- Steve
>
> Signed-off-by: Michael Sartain <[email protected]>
> ---
> trace-cmd-local.h | 2 +-
> trace-input.c | 8 ++++----
> trace-msg.c | 8 ++------
> 3 files changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/trace-cmd-local.h b/trace-cmd-local.h
> index 9412f9d..8595a8a 100644
> --- a/trace-cmd-local.h
> +++ b/trace-cmd-local.h
> @@ -31,7 +31,7 @@ static ssize_t __do_write(int fd, const void *data, size_t size)
> ssize_t w;
>
> do {
> - w = write(fd, data, size - tot);
> + w = TEMP_FAILURE_RETRY(write(fd, data, size - tot));
> tot += w;
>
> if (!w)
> diff --git a/trace-input.c b/trace-input.c
> index 89ddcf5..251d32b 100644
> --- a/trace-input.c
> +++ b/trace-input.c
> @@ -202,7 +202,7 @@ static ssize_t do_read(struct tracecmd_input *handle, void *data, size_t size)
> ssize_t r;
>
> do {
> - r = read(handle->fd, data, size - tot);
> + r = TEMP_FAILURE_RETRY(read(handle->fd, data, size - tot));
> tot += r;
>
> if (!r)
> @@ -774,7 +774,7 @@ static int read_page(struct tracecmd_input *handle, off64_t offset,
> off64_t ret;
>
> if (handle->use_pipe) {
> - ret = read(handle->cpu_data[cpu].pipe_fd, map, handle->page_size);
> + ret = TEMP_FAILURE_RETRY(read(handle->cpu_data[cpu].pipe_fd, map, handle->page_size));
> /* Set EAGAIN if the pipe is empty */
> if (ret < 0) {
> errno = EAGAIN;
> @@ -2645,7 +2645,7 @@ struct tracecmd_input *tracecmd_alloc(const char *file)
> {
> int fd;
>
> - fd = open(file, O_RDONLY);
> + fd = TEMP_FAILURE_RETRY(open(file, O_RDONLY));
> if (fd < 0)
> return NULL;
>
> @@ -2686,7 +2686,7 @@ struct tracecmd_input *tracecmd_open(const char *file)
> {
> int fd;
>
> - fd = open(file, O_RDONLY);
> + fd = TEMP_FAILURE_RETRY(open(file, O_RDONLY));
> if (fd < 0)
> return NULL;
>
> diff --git a/trace-msg.c b/trace-msg.c
> index 3991985..d358318 100644
> --- a/trace-msg.c
> +++ b/trace-msg.c
> @@ -291,10 +291,8 @@ static int msg_read(int fd, void *buf, u32 size, int *n)
> ssize_t r;
>
> while (size) {
> - r = read(fd, buf + *n, size);
> + r = TEMP_FAILURE_RETRY(read(fd, buf + *n, size));
> if (r < 0) {
> - if (errno == EINTR)
> - continue;
> return -errno;
> } else if (!r)
> return -ENOTCONN;
> @@ -662,10 +660,8 @@ int tracecmd_msg_collect_metadata(int ifd, int ofd)
> t = n;
> s = 0;
> do {
> - s = write(ofd, msg.data.meta.buf+s, t);
> + s = TEMP_FAILURE_RETRY(write(ofd, msg.data.meta.buf+s, t));
> if (s < 0) {
> - if (errno == EINTR)
> - continue;
> warning("writing to file");
> return -errno;
> }
On Wed, 14 Jun 2017 18:27:59 -0600
Michael Sartain <[email protected]> wrote:
> The tot variable in __do_write and do_read is incremented with the amount read
> / written, but subsequent times through the loop the calls continue to use the
> original data pointer.
>
> Signed-off-by: Michael Sartain <[email protected]>
> ---
> trace-cmd-local.h | 2 +-
> trace-input.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/trace-cmd-local.h b/trace-cmd-local.h
> index 8595a8a..b8ab35b 100644
> --- a/trace-cmd-local.h
> +++ b/trace-cmd-local.h
> @@ -31,7 +31,7 @@ static ssize_t __do_write(int fd, const void *data, size_t size)
> ssize_t w;
>
> do {
> - w = TEMP_FAILURE_RETRY(write(fd, data, size - tot));
> + w = TEMP_FAILURE_RETRY(write(fd, data + tot, size - tot));
Good catch. I'm going to modify this to remove the TEMP_FAILURE_RETRY()
though. I'll hopefully get this pushed out later today, and we could
add the write_intr() and friends later.
-- Steve
> tot += w;
>
> if (!w)
> diff --git a/trace-input.c b/trace-input.c
> index 251d32b..8395917 100644
> --- a/trace-input.c
> +++ b/trace-input.c
> @@ -202,7 +202,7 @@ static ssize_t do_read(struct tracecmd_input *handle, void *data, size_t size)
> ssize_t r;
>
> do {
> - r = TEMP_FAILURE_RETRY(read(handle->fd, data, size - tot));
> + r = TEMP_FAILURE_RETRY(read(handle->fd, data + tot, size - tot));
> tot += r;
>
> if (!r)
On Wed, Jun 21, 2017 at 09:29:14AM -0400, Steven Rostedt wrote:
> On Wed, 14 Jun 2017 18:27:59 -0600
> Michael Sartain <[email protected]> wrote:
>
> > The tot variable in __do_write and do_read is incremented with the amount read
> > / written, but subsequent times through the loop the calls continue to use the
> > original data pointer.
> >
> > Signed-off-by: Michael Sartain <[email protected]>
> > ---
> > trace-cmd-local.h | 2 +-
> > trace-input.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/trace-cmd-local.h b/trace-cmd-local.h
> > index 8595a8a..b8ab35b 100644
> > --- a/trace-cmd-local.h
> > +++ b/trace-cmd-local.h
> > @@ -31,7 +31,7 @@ static ssize_t __do_write(int fd, const void *data, size_t size)
> > ssize_t w;
> >
> > do {
> > - w = TEMP_FAILURE_RETRY(write(fd, data, size - tot));
> > + w = TEMP_FAILURE_RETRY(write(fd, data + tot, size - tot));
>
> Good catch. I'm going to modify this to remove the TEMP_FAILURE_RETRY()
> though. I'll hopefully get this pushed out later today, and we could
> add the write_intr() and friends later.
I've got a couple other patches ready to submit. I'll wait for your push,
rebase those, and add a TEMP_FAILURE_RETRY -> *_intr() patch with those.
Thanks much Steve.
-Mike
Commit-ID: 952a99ccfa9db2f9a32810fc9c0084f532dd871a
Gitweb: https://git.kernel.org/tip/952a99ccfa9db2f9a32810fc9c0084f532dd871a
Author: Michael Sartain <[email protected]>
AuthorDate: Thu, 11 Jan 2018 19:47:42 -0500
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 17 Jan 2018 10:21:39 -0300
tools lib traceevent: Fix bad force_token escape sequence
Older kernels have a bug that creates invalid symbols. event-parse.c
handles them by replacing them with a "%s" token. But the fix included
an extra backslash, and "\%s" was added incorrectly.
Signed-off-by: Michael Sartain <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Cc: Andrew Morton <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Link: http://lkml.kernel.org/r/d320000d37c10ce0912851e1fb78d1e0c946bcd9.1497486273.git.mikesart@fastmail.com
Signed-off-by: Steven Rostedt <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/event-parse.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 7ce724f..0bc1a6d 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -1094,7 +1094,7 @@ static enum event_type __read_token(char **tok)
if (strcmp(*tok, "LOCAL_PR_FMT") == 0) {
free(*tok);
*tok = NULL;
- return force_token("\"\%s\" ", tok);
+ return force_token("\"%s\" ", tok);
} else if (strcmp(*tok, "STA_PR_FMT") == 0) {
free(*tok);
*tok = NULL;