linux-kernel - Re: [PATCH] bitops: add equivalent of BIT(x) for bitfields

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: <20161205153400.6jrtczpudq6x5rff@pd.tnic>
Date:   Mon, 5 Dec 2016 16:34:00 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Sebastian Frias <sf84@...oste.net>
Cc:     zijun_hu <zijun_hu@....com>, Sasha Levin <sasha.levin@...cle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Mason <slash.tmp@...e.fr>,
        Maxime Coquelin <maxime.coquelin@...com>,
        Harvey Harrison <harvey.harrison@...il.com>
Subject: Re: [PATCH] bitops: add equivalent of BIT(x) for bitfields

On Mon, Dec 05, 2016 at 02:36:07PM +0100, Sebastian Frias wrote:
> + * Equivalent of BIT(x) but for contiguous bitfields
> + * SETBITFIELD(1, 0,0xff) = 0x00000003
> + * SETBITFIELD(3, 0,0xff) = 0x0000000f
> + * SETBITFIELD(15,8,0xff) = 0x0000ff00
> + * SETBITFIELD(6, 6,   1) = 0x00000040 == BIT(6)
> + */
> +#define SETBITFIELD(msb, lsb, val)     \
> +	(((val) << (lsb)) & (GENMASK((msb), (lsb))))
> +#define SETBITFIELD_ULL(msb, lsb, val) \
> +	(((val) << (lsb)) & (GENMASK_ULL((msb), (lsb))))

What is the use case for these macros?

I'm thinking it would be better if they were really generic so that I
can be able to hand in a value and to say, set bits from [lsb, msb] to
this bit pattern. Because then you'd have to punch a hole in the value
with a mask and then OR-in the actual mask you want to have.

I.e., something like this:

#define SET_BITMASK(val, msb, lsb, mask)        \
        (val = (val & ~(GENMASK(msb, lsb))) | (mask << lsb))

So that you can be able to do:

unsigned int val = 0x11110000;

SET_BITMASK(val, 19, 12, 0x5a)
val = 0x1115a000;

I'm not sure, though, whether this won't make the actual use too cryptic
and people having to go look it up each time they use it instead of
actually doing the ORing in by hand which everyone can understand from
staring at it instead of seeing a funny macro SET_BITMASK().

Actually, you could simply add another macro which is called

GENMASK_MASK(msb, lsb, mask)

or so (yeah, yucky name) which gives you that arbitrary mask which you
then can use anywhere.

Because then it becomes more readable:

        val &= ~GENMASK(19, 12);
        val |= GENMASK_MASK(19, 12, 0xa5);

Now you know exactly what happens: first line punches the hole, second
ORs in the new mask.

Hmmm...

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.