On Thu, 26 Oct 2017, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Thu, Oct 26, 2017 at 08:51:57PM +0200, Noralf Trønnes wrote:
>>
>> Den 26.10.2017 19.49, skrev Ville Syrjälä:
>> > On Thu, Oct 26, 2017 at 06:57:27PM +0200, Noralf Trønnes wrote:
>> >> Add drm_printf_indent() that adds tab indentation according to argument.
>> >>
>> >> Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
>> >> ---
>> >> drivers/gpu/drm/drm_print.c | 21 +++++++++++++++++++++
>> >> include/drm/drm_print.h | 2 ++
>> >> 2 files changed, 23 insertions(+)
>> >>
>> >> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
>> >> index 0b3bf476dc4b..dac3ee98b30f 100644
>> >> --- a/drivers/gpu/drm/drm_print.c
>> >> +++ b/drivers/gpu/drm/drm_print.c
>> >> @@ -64,6 +64,27 @@ void drm_printf(struct drm_printer *p, const char *f, ...)
>> >> }
>> >> EXPORT_SYMBOL(drm_printf);
>> >>
>> >> +/**
>> >> + * drm_printf_indent - print to a &drm_printer stream with indentation
>> >> + * @p: the &drm_printer
>> >> + * @i: indentation
>> >> + * @f: format string
>> >> + */
>> >> +void drm_printf_indent(struct drm_printer *p, unsigned int i, const char *f, ...)
>> >> +{
>> >> + struct va_format vaf;
>> >> + va_list args;
>> >> +
>> >> + drm_printf(p, "%.*s", i, "\t\t\t\t\t\t\t\t\t\t");
>> >> +
>> >> + va_start(args, f);
>> >> + vaf.fmt = f;
>> >> + vaf.va = &args;
>> >> + p->printfn(p, &vaf);
>> >> + va_end(args);
>> >> +}
>> >> +EXPORT_SYMBOL(drm_printf_indent);
>> > The double printf() is rather unfortunate. Sadly I don't think there's
>> > any proper way to manipulate a va_list to avoid that.
>> >
>> > Hmm. Would it work if we simply make it a macro? Eg.
>> >
>> > #define drm_printf_indent(printer, indent, fmt, ...) \
>> > drm_printf((printer), "%.*s" fmt, (indent), "\t\t\t", ##__VA_ARGS__)
>>
>> The macro worked fine and it looks like a better solution to me.
>
> Only downside is that it bloats every format string. But as long
> as there aren't a huge number of useless callers we should be fine.
>
>>
>> > The "\t\t\t..." thing is also rather off putting, but I guess it's
>> > the best we can do if we want to keep it to one printf(). And maybe we
>> > should have a check in there to make sure we have enought tabs in the
>> > string to satisfy the indent level, or we just clamp the indent level
>> > silently to something reasonable?
>>
>> I put 10 tabs in my suggestion, which should be enough and I think it's
>> OK to just silently fail to do more. If 10 isn't enough it's easy to add
>> more for the developer that hits the limit.
>
> 10 is probably even overkill. I'd say if you have to go past four or so
> then there's alredy a bigger problem in the code ;)
Just put some non-tab character at the end of the tab string? Dunno, +
or * or something, just to draw attention to it instead of just silently
failing to indent.
Overall I think this is a neat way to do indents.
BR,
Jani.
>
>>
>> Noralf.
>>
>> > Oh, seeing the \t now reminds me that tabs won't necesarily get printed
>> > out properly. At least I've seen fbcon just printing some weird blobs
>> > instead of tabs. Not sure if it's just a matter of having a crappy font
>> > or what. That said, the state dump code is using tabs already, so I guess
>> > this wouldn't make it worse at least.
>> >
>> >> +
>> >> #define DRM_PRINTK_FMT "[" DRM_NAME ":%s]%s %pV"
>> >>
>> >> void drm_dev_printk(const struct device *dev, const char *level,
>> >> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
>> >> index 7b9c86a6ca3e..73dcd16eca49 100644
>> >> --- a/include/drm/drm_print.h
>> >> +++ b/include/drm/drm_print.h
>> >> @@ -79,6 +79,8 @@ void __drm_printfn_debug(struct drm_printer *p, struct va_format *vaf);
>> >>
>> >> __printf(2, 3)
>> >> void drm_printf(struct drm_printer *p, const char *f, ...);
>> >> +__printf(3, 4)
>> >> +void drm_printf_indent(struct drm_printer *p, unsigned int i, const char *f, ...);
>> >>
>> >>
>> >> /**
>> >> --
>> >> 2.14.2
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
