svn commit: r358152 - head/bin/sh

[Conrad Meyer]

Given the report and looking at the change, I suspect the problem is
the promotion of '-residue' from size_t (unsigned 32-bit on i386) to
off_t (signed 64-bit).  Something like '-(off_t)residue' or even
'off_t residue;' should fix it.

Repro:

static void
myoff(off_t foo)
{
        printf("%jd\n", (intmax_t)foo);
}

void
main(int argc, char **argv)
{
        uint32_t sz = 1023;
        myoff(-sz);
}

Prints: "4294966273" (not: "-1023").  Either proposed fix above
produces the expected -1023.

Best,
Conrad

On Fri, Feb 21, 2020 at 1:53 PM Li-Wen Hsu <lwhsu at freebsd.org> wrote:
>
> On Sat, Feb 22, 2020 at 4:58 AM Antoine Brodin <antoine at freebsd.org> wrote:
> >
> > On Thu, Feb 20, 2020 at 4:01 AM Hiroki Sato <hrs at freebsd.org> wrote:
> > >
> > > Author: hrs
> > > Date: Thu Feb 20 03:01:27 2020
> > > New Revision: 358152
> > > URL: https://svnweb.freebsd.org/changeset/base/358152
> > >
> > > Log:
> > >   Improve performance of "read" built-in command when using a seekable
> > >   fd.
> > >
> > >   The read built-in command calls read(2) with a 1-byte buffer because
> > >   newline characters need to be detected even on a byte stream which
> > >   comes from a non-seekable file descriptor.  Because of this, the
> > >   following script calls >6,000 read(2) to show a 6KiB file:
> > >
> > >    while read IN; do echo "$IN"; done < /COPYRIGHT
> > >
> > >   When the input byte stream is seekable, it is possible to read a data
> > >   block and then reposition the file pointer to where a newline
> > >   character found.  This change adds a small buffer to do this and
> > >   reduces the number of read(2) calls.
> > >
> > >   Theoretically, multiple built-in commands reading the same seekable
> > >   byte stream in a single pipe chain can share the buffer.  However,
> > >   this change just makes a single invocation of the read built-in
> > >   allocate a buffer and deallocate it every time for simplicity.
> > >   Although this causes read(2) to read the same regions multiple times,
> > >   the performance penalty should be small compared to the reduction of
> > >   read(2) calls.
> > >
> > >   Reviewed by:          jilles
> > >   MFC after:            1 week
> > >   Differential Revision:        https://reviews.freebsd.org/D23747
> >
> > This seems to be broken on at least i386.
> > Please either fix or revert.
> >
> > Antoine (with hat: portmgr)
>
> Could you provide more detail?  I'm worried because I didn't see
> related regression from the recent test results. We may need to add
> more test against the breakage you mentioned.
>
> Thanks,
> Li-Wen