On Wed, 14 Jun 2017 18:27:58 -0600
Michael Sartain <mikes...@fastmail.com> 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 <mikes...@fastmail.com>
> ---
> 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;
> }
