Re: [PATCH v3 16/18] rtla/trace: Fix I/O handling in save_trace_to_file()

[Wander Lairson Costa <wander@redhat.com>]

On Wed, Mar 04, 2026 at 11:30:13AM +0100, Tomas Glozar wrote:
> čt 15. 1. 2026 v 18:29 odesílatel Wander Lairson Costa
> <wander@redhat.com> napsal:
> > diff --git a/tools/tracing/rtla/src/trace.c b/tools/tracing/rtla/src/trace.c
> > index fed3362527b08..8e93b48d33ef8 100644
> > --- a/tools/tracing/rtla/src/trace.c
> > +++ b/tools/tracing/rtla/src/trace.c
> > @@ -73,6 +73,7 @@ int save_trace_to_file(struct tracefs_instance *inst, const char *filename)
> >         char buffer[4096];
> >         int out_fd, in_fd;
> >         int retval = -1;
> > +       ssize_t n_read;
> >
> >         if (!inst || !filename)
> >                 return 0;
> > @@ -90,15 +91,30 @@ int save_trace_to_file(struct tracefs_instance *inst, const char *filename)
> >                 goto out_close_in;
> >         }
> >
> > -       do {
> > -               retval = read(in_fd, buffer, sizeof(buffer));
> > -               if (retval <= 0)
> > +       for (;;) {
> > +               n_read = read(in_fd, buffer, sizeof(buffer));
> > +               if (n_read < 0) {
> > +                       if (errno == EINTR)
> > +                               continue;
> > +                       err_msg("Error reading trace file: %s\n", strerror(errno));
> >                         goto out_close;
> > +               }
> > +               if (n_read == 0)
> > +                       break;
> >
> > -               retval = write(out_fd, buffer, retval);
> > -               if (retval < 0)
> > -                       goto out_close;
> > -       } while (retval > 0);
> > +               ssize_t n_written = 0;
> 
> Why break the style of declaring all variables at the beginning of the
> function? n_read, added in the same commit, keeps the style.
> 
> This also applies to the previous patch.

In this case `n_written` is harmless and I see no problem in moving it to the
beginning of the function.

For `written` and `w`, declaring them where they are right now brings the
benefit of making them const correct. If we move them to the top of the
function, we lose that. Since the kernel moved to gnu11, the C89 top-of-block
declarations are relaxed. Keeping them at the point of initialization minimizes
their scope and documents the intent. This helps avoid programmer mistakes.

However, I fail to follow my own advice more than I am willing to admit.

> 
> Tomas
>