On Tue, 19 Jun 2018, Andy Shevchenko wrote:
> On Sun, Jun 17, 2018 at 10:07 PM, Nicolas Pitre
> <nicolas.pi...@linaro.org> wrote:
> > Make sure the unicode screen buffer matches the video screen content.
> > This is provided for debugging convenience and disabled by default.
>
> > +#define VC_UNI_SCREEN_DEBUG 0
>
> > + if (!VC_UNI_SCREEN_DEBUG || !uniscr)
> > + return;
>
> Hmm... Interesting. I would rather go with
> #ifdef ..._DEBUG
> ...
> #else
> return;
> #endif
>
> It will relax requirement on how to define _DEBUG. I don't recall I
> see something like you proposing in the kernel for the same purpose.
Some random examples:
include/crypto/scatterwalk.h:106;
if (ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE && !PageSlab(page))
include/math-emu/single.h:68:
if (!FP_INHIBIT_RESULTS)
This form also allows for the compiler to parse and validate the code
whether or not the feature is enabled, while still optimizing it away in
the end if not enabled.
> > +
> > + WARN_CONSOLE_UNLOCKED();
> > +
> > + /*
> > + * Make sure our unicode screen translates into the same glyphs
> > + * as the actual screen. This is brutal indeed.
> > + */
> > + p = (unsigned short *)vc->vc_origin;
> > + mask = vc->vc_hi_font_mask | 0xff;
> > + for (y = 0; y < vc->vc_rows; y++) {
> > + char32_t *line = uniscr->lines[y];
> > + for (x = 0; x < vc->vc_cols; x++) {
> > + u16 glyph = scr_readw(p++) & mask;
> > + char32_t uc = line[x];
> > + int tc = conv_uni_to_pc(vc, uc);
> > + if (tc == -4)
> > + tc = conv_uni_to_pc(vc, 0xfffd);
> > + if (tc == -4)
> > + tc = conv_uni_to_pc(vc, '?');
> > + if (tc != glyph)
>
> > + pr_notice("%s: mismatch at %d,%d: "
> > + "glyph=%#x tc=%#x\n", __func__,
> > + x, y, glyph, tc);
>
> Don't split format string in printk(). checkpatch will not warn on longer
> lines.
I didn't do it like that for checkpatch but to keep the code readable.
I don't particularly care either ways though.
Nicolas
