[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 1 Dec 2018 13:42:30 +1100
From: Xan Phung <xan.phung@...il.com>
To: musl@...ts.openwall.com
Subject: Re: stdio glitch & questions
On Sat, 1 Dec 2018 at 11:02, Rich Felker <dalias@...c.org> wrote:
>
> I've been trying to understand what you're trying to do. It seems you
> chose to work at the point of line-buffered flush logic, since that
> happens to be the only case where f->write is called with an argument
> that might fit in the remaining buffer space.
>
>
Yes that's correct... Also, the other reason I chose __fwrite.c is it's the
only place where the search for '\n' is done.
An additional objective I had was to split the loop searching for '\n' into
two stages:
(i) Stage 1: search for '\n' word by word ie: 8 bytes at a time ... if '\n'
found at >~16 bytes before start of "s" array then go straight to f->write
without copying bytes
(I don't show it in my code, but the algorithm to do stage 1 would be
similar to memchr.c)
(ii) Stage 2: in final 9~16 bytes, drop down into byte-by-byte searching
for '\n' (also doing copy of residual bytes into buffer when '\n' found)
> As written the alignment logic and pointer arithmetic is invalid; the
> sums/differences are out of bounds of the array, and i<=0 is not
> meaningful since i has an unsigned type (and so does l+s-t). But even
> if it could be made correct, it's all completely unnecessary and just
> making the code slower and less readable. If __fwritex were the right
> place for this code, all you would need to
>
do is check whether i<16 (or whatever threshold) before calling
> f->write, and if so, memcpy'ing it to the buffer then calling f->write
> with a length of 0.
I agree, the check for i <= X is a simpler way of expressing the algorithm.
[X is a value between 9 and 16 that guarantees (s + X) will be word aligned]
In my version, I introduced a new array base "t" and rebase i to index into
"t" because "t" is a guaranteed word aligned pointer.
However, this word alignment of "t" is not exploited in the current
byte-at-a-time searching for '\n', so yes at present it's unnecessary.
> However, then you could not use the return value
> of f->write to determine if it succeeded (see how fflush and fseek
> have to deal with this case). Contrary to what your code assumes,
> f->write does not (and cannot, since the type is unsigned) return a
> negative value on error.
>
>
Ok, noted, the expression to check for error should be (!f->wpos) and not n
< 0
Instead, I think it probably makes more sense to put the logic in
> __stdio_write, but this will also be somewhat nontrivial to work in.
> At least the "iovcnt == 2 ? ..." logic needs to be adapted to
> something like "rem > len ? ...". Before the loop should probably be
> something like "if (len < f->wend-f->wpos && len <= 16) ..." to
> conditionally copy the new data into the buffer.
>
> Do you see any reason to prefer doing it in __fwritex?
>
> Wouldn't putting check for i < X in __fwrite.c better because it:
(a) facilitates word-by-word searching objective outlined above
(b) check for space in buffer is already done in line 10 of __fwrite.c,
hence avoids need for any more buffer length checks
(c) ?? speeds up writes which doesn't use __stdio_write
Content of type "text/html" skipped
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
