Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Fri, 15 Mar 2019 09:27:06 +0100 | Subject | Re: [PATCH 1/5] lib/sort: Make swap functions more generic |
| |
Hi George,
On Fri, Mar 15, 2019 at 4:36 AM George Spelvin <lkml@sdf.org> wrote: > >> 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.
Argument of least surprise: don't call something a duck if it's not guaranteed to behave like a duck.
If I read "long", this triggers a warning flag in my head: be careful, this is 32-bit on 32-bit platforms, and 64-bit on 64-bit platforms.
There's a reason the newer I/O ioread{8,16,32} accessors use explicit sizes, unlike the ancient x86-centric read[bwl]().
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
| |