[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] tail: remove excessive checks on buffer sizes before printin
From: |
Nikolay Nechaev |
Subject: |
Re: [PATCH] tail: remove excessive checks on buffer sizes before printing |
Date: |
Sun, 20 Jun 2021 23:57:37 +0300 |
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