Re: [PATCH] tail: remove excessive checks on buffer sizes before printing

[Nikolay Nechaev]

On Sunday, 20 June 2021 23:19:12 MSK you wrote:
> On 20/06/2021 15:54, Nikolay Nechaev wrote:
> > * src/tail.c: remove excessive size checks before calls to
> > `xwrite_stdout`
> > 
> > `xwrite_stdout` itself checks if what is to be printed out
> > has positive size, and only proceeds then. There is no need
> > to check if buffers are of positive size before printing
> > them out with `xwrite_stdout`
> > 
> > Signed-off-by: Nikolay Nechaev <Nikolay_Nechaev@mail.ru>
> > ---
> >   src/tail.c | 11 +++++------
> >   1 file changed, 5 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/tail.c b/src/tail.c
> > index ff567560d..44a6e3e68 100644
> > --- a/src/tail.c
> > +++ b/src/tail.c
> > @@ -579,8 +579,7 @@ file_lines (char const *pretty_filename, int fd, 
> > uintmax_t n_lines,
> >               {
> >                 /* If this newline isn't the last character in the buffer,
> >                    output the part that is after it.  */
> > -              if (n != bytes_read - 1)
> > -                xwrite_stdout (nl + 1, bytes_read - (n + 1));
> > +              xwrite_stdout (nl + 1, bytes_read - (n + 1));
> >                 *read_pos += dump_remainder (false, pretty_filename, fd,
> >                                              end_pos - (pos + bytes_read));
> >                 return true;
> > @@ -881,8 +880,8 @@ start_bytes (char const *pretty_filename, int fd, 
> > uintmax_t n_bytes,
> >         else
> >           {
> >             size_t n_remaining = bytes_read - n_bytes;
> > -          if (n_remaining)
> > -            xwrite_stdout (&buffer[n_bytes], n_remaining);
> > +          // Print extra characters if there are any
> > +          xwrite_stdout (&buffer[n_bytes], n_remaining);
> >             break;
> >           }
> >       }
> > @@ -923,8 +922,8 @@ start_lines (char const *pretty_filename, int fd, 
> > uintmax_t n_lines,
> >             ++p;
> >             if (--n_lines == 0)
> >               {
> > -              if (p < buffer_end)
> > -                xwrite_stdout (p, buffer_end - p);
> > +              // Print extra characters if there are any
> > +              xwrite_stdout (p, buffer_end - p);
> >                 return 0;
> >               }
> >           }
> > 
> This guard has been internal to the function since 1994:
> https://github.com/coreutils/coreutils/commit/32340b45e
> 
> So all guards that check != 0 are redundant.
> However those that check >= 0 are not.
> The last one (in start_lines()) is such a case.
> I.e. p can be == buffer_end, so ++p == buffer_end +1,
> so buffer_end - p == -1.
> 
> I will apply the first two chunks if you agree.
> 
> thanks,
> Pádraig
> 

Oh, I didn't notice that `n_bytes` in `xwrite_stdout` is of
type `size_t`, which can't be negative. You're right.

> I will apply the first two chunks if you agree.

Yes, sure!

BTW, it might be confusing that `xwrite_stdout` has a line
starting with `if(n_bytes > 0`, which is, because of the
variable's type, equivalent to `if(n_bytes == 0`

-- 
Best wishes,
Nikolay Nechaev