lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <nycvar.YSQ.7.76.1806182110450.16670@knanqh.ubzr>
Date:   Mon, 18 Jun 2018 21:50:08 -0400 (EDT)
From:   Nicolas Pitre <nicolas.pitre@...aro.org>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Dave Mielke <Dave@...lke.cc>,
        Samuel Thibault <samuel.thibault@...-lyon.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 4/4] vt: coherence validation code for the unicode
 screen buffer

On Tue, 19 Jun 2018, Andy Shevchenko wrote:

> On Sun, Jun 17, 2018 at 10:07 PM, Nicolas Pitre
> <nicolas.pitre@...aro.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ