linux-kernel - Re: [PATCH 1/5] lib/sort: Make swap functions more generic

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]

Date:   Fri, 15 Mar 2019 03:35:30 GMT
From:   George Spelvin <lkml@....org>
To:     andriy.shevchenko@...ux.intel.com, lkml@....org, st5pub@...dex.ru
Cc:     akpm@...ux-foundation.org, daniel.wagner@...mens.com,
        dchinner@...hat.com, don.mullis@...il.com, geert@...ux-m68k.org,
        linux-kernel@...r.kernel.org, linux@...musvillemoes.dk
Subject: Re: [PATCH 1/5] lib/sort: Make swap functions more generic

>> swap_bytes / swap_4byte_words / swap_8byte_words
>> swap_bytes / swap_ints / swap_longs
>> swap_1 / swap_4 / swap_8
>> Pistols at dawn?

On Thu, 14 Mar 2019 at 22:59:55 +0300, Andrey Abramov wrote:
> Yes, in my opinion, swap_bytes / swap_ints / swap_longs are the
> most readable because we have both swap_ints and swap_longs functions
> (in one file near each other), so I don't think that there will be
> any confusion about size.

Yes, that's what I thought.  They're three related but different
functions, suffixed _bytes, _ints, and _longs.  What could the
difference possibly be?  And if anyone has any lingering doubts,
the functions are right there, with exquisitely clear comments.

No to mention where they're used.  Is "is_aligned(base, size, 8)"
remotely obscure?  Especially in context:

		if (is_aligned(base, size, 8))
			swap_func = swap_longs;
		else if (is_aligned(base, size, 4))
			swap_func = swap_ints;
		else
			swap_func = swap_bytes;

What subtle and mysterious code.

> But actually, it doesn't matter which name will you take, because
> the meaning of each, in my opinion, is obvious enough, so I don't
> mind about any of these options.

I'm just amazed that this piece of bikeshedding is the most
contentious thing about the patch series.

I mean, if I'd named them:
	llanfairpwllgwyngyll()
	shravanabelagola()
	zheleznodorozhny()
or
	peckish()
	esuriant()
	hungry()
then yes, those would be bad names.

I prefer the shorter _ints and _longs names, but this is just
not a hill I want to die on.