| Message ID | 20220620004233.3805-14-kent.overstreet@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Printbufs - new data structure for building strings | expand |
On 20/06/2022 02.42, Kent Overstreet wrote: > The current test code checks that sprintf never writes past the > terminating nul. This is a rather strange requirement, completely > separate from writing past the end of the buffer, which of course we > can't do And which of course you don't check anymore. So any statement about "all tests passing after these patches" is not worth much. So NAK in this form, but perhaps if/when my year-old patch gets picked up (or you could include it in the series) we can talk about eliminating the check for past-nul-before-end-of-buffer writes. Rasmus
On Tue, Jun 21, 2022 at 09:19:55AM +0200, Rasmus Villemoes wrote: > On 20/06/2022 02.42, Kent Overstreet wrote: > > The current test code checks that sprintf never writes past the > > terminating nul. This is a rather strange requirement, completely > > separate from writing past the end of the buffer, which of course we > > can't do > > And which of course you don't check anymore. So any statement about "all > tests passing after these patches" is not worth much. > > So NAK in this form, but perhaps if/when my year-old patch gets picked > up (or you could include it in the series) we can talk about eliminating > the check for past-nul-before-end-of-buffer writes. I'm more than happy to pick up your patch, please send it to me.
diff --git a/lib/test_printf.c b/lib/test_printf.c index e3de52da91..853e89e2f8 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -79,12 +79,6 @@ do_test(int bufsize, const char *expect, int elen, return 1; } - if (memchr_inv(test_buffer + written + 1, FILL_CHAR, BUF_SIZE + PAD_SIZE - (written + 1))) { - pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n", - bufsize, fmt); - return 1; - } - if (memcmp(test_buffer, expect, written)) { pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'\n", bufsize, fmt, test_buffer, written, expect);
The current test code checks that sprintf never writes past the terminating nul. This is a rather strange requirement, completely separate from writing past the end of the buffer, which of course we can't do: writing anywhere to the buffer passed to snprintf, within size of course, should be perfectly fine. Since this check has no documentation as to where it comes from or what depends on it, and it's getting in the way of further refactoring (printf_spec handling is right now scattered massively throughout the code, and we'd like to consolidate it) - delete it. Also, many current pretty-printers building up their output on the stack, and then copy it to the actual output buffer - by eliminating this requirement we can kill those extra buffers. Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com> --- lib/test_printf.c | 6 ------ 1 file changed, 6 deletions(-)