Re: [PATCH v3 1/5] seq_file: provide an analogue of print_hex_dump()
From: Andy Shevchenko
Date: Mon Sep 01 2014 - 06:16:08 EST
On Mon, Sep 1, 2014 at 12:25 PM, Geert Uytterhoeven
> Hi Andy,
> On Mon, Sep 1, 2014 at 11:09 AM, Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>> On Mon, 2014-09-01 at 10:58 +0200, Geert Uytterhoeven wrote:
>>> On Mon, Sep 1, 2014 at 10:36 AM, Andy Shevchenko
>>> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>>> >> ... and extra copying for no good reason. Why not check that we have
>>> >> enough space in buffer and generate directly into it? See what e.g.
>>> >> seq_escape() is doing...
>>> > What about this variant?
>>> I think it needs a call to seq_set_overflow() in case the buffer is too small,
>>> so the caller will retry with a bigger buffer.
>> Yes, in two places it would be useful to do.
> Two places? I see only one, just before calling hex_dump_to_buffer.
seq_putc doesn't set it as I can see.
>> But what the condition for "buffer is too small", the same groupsize * 2
>> + 1 or you mean something else?
> "groupsize * 2 + 1" is not the amount of bytes hex_dump_to_buffer() wants
> to write. It's only the size for one word.
> You could check if there are at least "32 * 3 + 2 + 32 + 1" bytes (your
> old linebuf size) available.
This is a good question why this number? What if we have to print only
one byte (as different groupsize)?
I think the requirement for one groupsize is quite okay.
> However, to protect against overflows if hex_dump_to_buffer() ever changes,
> I think it would be better to let hex_dump_to_buffer() indicate if the
> passed buffer was to small (it already checks the passed linebuflen).
> Then you can just check for that.
I thought about that. We may introduce either new call and make
current one the user of it or change all occurrences.
Nevertheless, currently it will print only one groupsize if there is
enough room for it but for two or more.
Thus, I prefer to keep the behaviour "print until we can".
With Best Regards,
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/