On Mon, Feb 23 2015, Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>> >> > So, why couldn't we split this to separate test case? It seems I already
>> >> > pointed this out.
>> >> >
>> >>
>> >> This actually provides better coverage
>> >
>> > I do not see much advantage of doing so. You may create a loop with
>> > random number for in-size and check. So, I prefer to see separate case
>> > for that.
>>
>> It's not about the size, it's about exercising all the various escape_*
>> helpers, to ensure that they all respect the end of the buffer, while
>> still returning the correct would-be output size. For that, one needs to
>> call string_escape_mem with various combinations of flags and input
>> buffers. The logic for that is already in place in test_string_escape
>> and its caller, and I see no point in duplicating all that.
>
> Thanks for clarification.
>
>> If you insist on a separate function for doing the overflow testing,
>> I'll just rip it out from my code and let you add such a test later.
>
> What about to make it a separate function *and* call from inside of
> test_string_escape? Would it work for you?
See my earlier point about "quite a lot of state to pass". But if this
static __init void
test_string_escape_overflow(const char *in, int p, char *out_real, int out_size,
unsigned int flags, const char *esc, int q_test,
const char *name)
{
int q_real;
memset(out_real, 'Z', out_size);
q_real = string_escape_mem(in, p, out_real, 0, flags, esc);
if (q_real != q_test)
pr_warn("Test '%s' failed: flags = %u, osz = 0, expected %d, got %d\n",
name, flags, q_test, q_real);
if (memchr_inv(out_real, 'Z', out_size))
pr_warn("Test '%s' failed: osz = 0 but string_escape_mem wrote to the buffer\n",
name);
}
is what you want, sure, have it your way.
I need to fix fs/proc/array.c in 3/3 as well, to make the kernel
compile+boot and make the series bisectable. Before I send v4 please let
me know what you think about this (the minimal fix I could come up with):
diff --git a/fs/proc/array.c b/fs/proc/array.c
index 1295a00ca316..20f2d50e2dba 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -99,10 +99,9 @@ static inline void task_name(struct seq_file *m, struct task_struct *p)
buf = m->buf + m->count;
/* Ignore error for now */
- string_escape_str(tcomm, &buf, m->size - m->count,
- ESCAPE_SPACE | ESCAPE_SPECIAL, "\n\\");
+ m->count += string_escape_str(tcomm, buf, m->size - m->count,
+ ESCAPE_SPACE | ESCAPE_SPECIAL, "\n\\");
- m->count = buf - m->buf;
seq_putc(m, '\n');
}
[Longer-term I think it would be a lot better not to poke around in
the internals of struct seq_file. One way is to do the escaping into a
stack buffer (2*sizeof(p->comm) should be enough) and then use something
like seq_write(m, buffer, min(sizeof(buffer),
return-value-from-string_escape_str)).
Another option is to do everything with a single seq_printf call,
something like
seq_printf(m, "Name:\t%*pEcs\n, (int)strlen(tcomm), tcomm)
That will escape more than just \ and \n, but that would IMO be an
improvement. But of course this is out of scope for this series.]
Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
