Re: [PATCH v3 1/5] seq_file: provide an analogue of print_hex_dump()

[Andy Shevchenko]

On Mon, 2014-09-01 at 12:59 +0200, Geert Uytterhoeven wrote:
> Hi Andy,
> 
> On Mon, Sep 1, 2014 at 12:15 PM, Andy Shevchenko
> <andy.shevchenko@xxxxxxxxx> wrote:
> >>>> 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 don't think complaining about a too-small buffer prematurely hurts.
> 
> > I think the requirement for one groupsize is quite okay.
> 
> Then you will loose data if the buffer is too small.
> 
> >> 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".
> 
> The idea of seq_*() is that it will retry with a bigger bufsize if there's
> not enough space.

Fair enough.

So, summarize above, the check before hex_dump_to_buffer() should care
about maximum possible buffer needed for one line. But we have already
rowsize calculated (we actually can't rely on groupsize in this case).

Do you agree with formula rowsize * 3 + 2 + rowsize + 1?

-- 
Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
Intel Finland Oy

--
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/