linux-kernel - Re: [PATCHv2 2/3] x86, ptdump: Simplify page flag evaluation 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: <CA+rthh_UJWqeTJP9aewuC0HRo9wnucy5GScqYH0=0ob=bn4nBw@mail.gmail.com>
Date:	Sun, 21 Sep 2014 22:33:45 +0200
From:	Mathias Krause <minipli@...glemail.com>
To:	Arjan van de Ven <arjan@...ux.intel.com>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	x86-ml <x86@...nel.org>
Subject: Re: [PATCHv2 2/3] x86, ptdump: Simplify page flag evaluation code

On 21 September 2014 21:49, Arjan van de Ven <arjan@...ux.intel.com> wrote:
> On 9/21/2014 8:26 AM, Mathias Krause wrote:
>>
>> -               if (pr & _PAGE_PCD)
>> -                       pt_dump_cont_printf(m, dmsg, "PCD ");
>> -               else
>> -                       pt_dump_cont_printf(m, dmsg, "    ");
>> +               pt_dump_cont(m, dmsg, "%-4s", pr & _PAGE_USER ? "USR" :
>> "");
>
>
> while you have some nice cleanups in your patch, I can't say I consider this
> an improvement.
> Yes the C standard allows ? to be used like this
> but no, I don't think it improves readability in general.

Not in general, but in this case, it does, IMHO.

> (I think for me the main exception is NULL pointer cases, but this is not
> one of these)

Apparently such a pattern (using the question mark operator combined
with a bit test to choose string constants) is used quite often in the
linux kernel as a simple grep tells me (probably catches a few false
positives but still a representative number):

$ git grep '[^&]&[^&].*? *"' | wc -l
2668

And, honestly, the bit test combined with the question mark operator
makes the code way more readable for me. It not only makes the code
more compact (1 instead of 4 lines). It also allows to have the common
parts written only once and thereby removing the possibility of having
them conflict with each other, e.g. make them generate strings of
different lengths.

Would you prefer the bit test to be surrounded by braces to make it
more easy to understand? Even though, the operator precedence is
clearly defined by the C standard for this case, so no braces are
needed.


Thanks,
Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/