linux-kernel - Re: [PATCH v2] leds: ledtrig-morse: send out morse code

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: <20180704024112.GB9015@1wt.eu>
Date:   Wed, 4 Jul 2018 04:41:12 +0200
From:   Willy Tarreau <w@....eu>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Andreas Klinger <ak@...klinger.de>,
        Jacek Anaszewski <jacek.anaszewski@...il.com>,
        Pavel Machek <pavel@....cz>, ben.whitten@...il.com,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Philippe Ombredanne <pombredanne@...b.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux LED Subsystem <linux-leds@...r.kernel.org>
Subject: Re: [PATCH v2] leds: ledtrig-morse: send out morse code

On Tue, Jul 03, 2018 at 09:43:06PM +0300, Andy Shevchenko wrote:
> > +struct morse_char {
> > +       char    c;
> > +       char    *z;
> > +};
> > +
> 
> > +static struct morse_char morse_table[] = {
> 
> const ?
> 
> > +       {'a', ".-"},
> > +       {'b', "-..."},
> > +       {'c', "-.-."},
> > +       {'d', "-.."},
> > +       {'e', "."},
> > +       {'f', "..-."},
> > +       {'g', "--."},
> > +       {'h', "...."},
> > +       {'i', ".."},
> > +       {'j', ".---"},
> > +       {'k', "-.-"},
> > +       {'l', ".-.."},
> > +       {'m', "--"},
> > +       {'n', "-."},
> > +       {'o', "---"},
> > +       {'p', ".--."},
> > +       {'q', "--.-"},
> > +       {'r', ".-."},
> > +       {'s', "..."},
> > +       {'t', "-"},
> > +       {'u', "..-"},
> > +       {'v', "...-"},
> > +       {'w', ".--"},
> > +       {'x', "-..-"},
> > +       {'y', "-.--"},
> > +       {'z', "--.."},
> > +       {'1', ".----"},
> > +       {'2', "..---"},
> > +       {'3', "...--"},
> > +       {'4', "....-"},
> > +       {'5', "....."},
> > +       {'6', "-...."},
> > +       {'7', "--..."},
> > +       {'8', "---.."},
> > +       {'9', "----."},
> > +       {'0', "-----"},
> 
> Do you expect this to be changed somehow?
> Otherwise we might just to keep two char arrays of alphas and digits
> in an order of ascii appearance.
> 
> In the code something like
> 
> ch = tolower(x);
> if (isalpha(ch))
>  code = alphas[ch - 'a'];
> else if (isdigit(ch))
>  code = digits[ch - '0'];
> else
>  code = unknown;
> 
> > +       {0, NULL},
> 
> And this will gone, you just provide it with known size,

Well, in this case it's even possible to go further and avoid storing
36 strings. Indeed, no representation is longer than 5 symbols, so you
can use 5 bits for the encoding (0=".", 1="-") and 3 bits for the
length, it gives you a single byte per character instead of a pointer
to a string plus 6 chars. Then in order to make it readable, 5 macros
can be provided to emit the code :

#define MORSE1(a,b)       (1 | ((a)<<3))
#define MORSE2(a,b)       (2 | ((a)<<3)|((b)<<4))
#define MORSE3(a,b,c)     (3 | ((a)<<3)|((b)<<4)|((c)<<5))
#define MORSE4(a,b,c,d)   (4 | ((a)<<3)|((b)<<4)|((c)<<5)|((d)<<6))
#define MORSE5(a,b,c,d,e) (5 | ((a)<<3)|((b)<<4)|((c)<<5)|((d)<<6)|((e)<<7))

Then all chars may be defined like this :

    ['a'] = MORSE2(0,1),
    ['b'] = MORSE4(1,0,0,0),
    ['c'] = MORSE4(1,0,1,0),
    ['d'] = MORSE3(1,0,0),
    ['e'] = MORSE1(0),
    ...

and when processing these :

    code = morse_table[tolower(c)];
    code_len = code & 7;
    code >>= 3;

    while (code_len) {
       if (code & 1)
           emit_long();
       else
           emit_short();
       code >>= 1;
       code_len--;
    }

In this case it could even cover the whole ASCII table at once since it's
not certain that the saved bytes compensate for the extra code and alignment
used to save them :-)

Note that I'm not suggesting that it is required to proceed like this, but
I think it makes the whole code more compact, which aligns with the purpose
of focusing on embedded devices.

Cheers,
Willy