Recently, Linus Torvalds publicly dismissed a RISC-V code contribution to the Linux kernel made by a Google engineer as “garbage code”:
https://lkml.org/lkml/2025/8/9/76
First, I think Linus should be more respectful of other people.
In addition, let’s focus on the make_u32_from_two_u16() helper. My understanding is that this is a C preprocessor macro (as the Linux Kernel is mainly written in C). Let’s compare that helper with the explicit code “(a << 16) + b” proposed by Linus.
First, this explicit code is likely wrong, and in fact Linus adds that “maybe you need to add a cast”.
Why should we add a cast? In Linus’s words: “[…] to make sure that ‘b’ doesn’t have high bits that pollutes the end result”. So, what should the explicit code look like according to him? “(a << 16) + (uint16_t)b”?
But let’s do a step back. We should ask ourselves: What are the types of ‘a’ and ‘b’? From the helper’s name, I would think they are two “u16”, so two uint16_t.
If I was asked to write C code that takes two uint16_t values ‘a’ and ‘b’ as input and combines them into a uint32_t, I would write something like that:
((uint32_t)a << 16) | (uint32_t)b
I would use the bitwise OR (|) instead of +; I find it more appropriate as we are working at the bit manipulation level here. But maybe that’s just a matter of personal preference and coding style.
Moreover, I’d use the type casts as shown above, on both ‘a’ and ‘b’.
I’m not sure what Linus meant with ‘b’ potentially having “high bits that pollutes the end result”. Could ‘b’ be a uint32_t? In that case, I would use a bitmask like 0xFFFF with bitwise AND (&) to clear the high bits of ‘b’.
Moreover, I’d probably use better names for ‘a’ and ‘b’, too, like ‘high’ and ‘low’, to make it clear what is the high 16-bit word and what is the low 16-bit word.
So, the correct explicit code is not something as simple as “(a << 16) + b”. You may need to type cast, and you have to pay attention to do it correctly with proper use of parentheses. And you may potentially need to clear the high bits of ‘b’ with a bitmask?
And, if this operation of combining two uint16_t into a uint32_t is done in several places, you sure have many opportunities to introduce bugs with the explicit code that Linus advocates for in his email!
So, it would be much better, clearer, nicer, and safer, to raise the semantic level of the code, and write a helper function or macro to do that combination safely and correctly.
A C macro could look like this:
#include <stdint.h>
#define MAKE_U32_FROM_TWO_U16(high, low) \
( ((uint32_t)(high) << 16) | (uint32_t)(low) )
Should we take into consideration the case in which ‘low’ has higher bits to clear? Then the macro becomes something like this:
#define MAKE_U32_FROM_TWO_U16(high, low) \
( ((uint32_t)(high) << 16) | ((uint32_t)(low) & 0xFFFF))
As you can see, the type casts, the parentheses, the potential bit-masking, do require attention. But once you get the code right, you can safely and conveniently reuse it every time you need!
So, the real garbage code is actually repeatedly writing explicit bug-prone or wrong code, like “(a << 16) + b”! Not hiding such code in a sane helper macro (or function), like shown above.
Instead of a preprocessor macro, we could use an inline helper function. For example, in C++ we could write something like this:
#include <stdint.h>
inline uint32_t make_u32_from_two_u16(uint16_t high, uint16_t low)
{
return (static_cast<uint32_t>(high) << 16) |
static_cast<uint32_t>(low);
}
We could even further refine this function, marking it noexcept, as it’s guaranteed to not throw exceptions.
And we could also make the function constexpr, as it can be evaluated at compile-time when the input arguments are constant.
With these additional refinements, we get:
inline constexpr uint32_t make_u32_from_two_u16(
uint16_t high,
uint16_t low) noexcept
{
return (static_cast<uint32_t>(high) << 16) |
static_cast<uint32_t>(low);
}
None of this addresses what Mr. Torvalds’ sole concern seems to be, namely that the “helper” obscures which variable is the low word and which is the high word.
How would you design this helper so that one can quickly see at the point of use which variable is the high word and which is the low word?
LikeLike
None of this addresses my points about writing explicit code that can be very bug prone because you can make mistakes in (mis)using parentheses for the type cast, or make mistakes about clearing the high bits of ‘b’ with a bitmask, etc. I won’t repeat my blog post here.
Talking about “obscuring parameters” and “which variable is the low word and which is the high word”, when you call a function (or macro) you are supposed to read the documentation. If you use a decent IDE, you can even hover over a function call and have the function declaration shown with parameter names. So, if you wrote the function declaration correctly, with proper parameter names like “high” and “low” instead of “a” and “b”, you can simply figure out at the call site what argument is the high word and what argument is the low word.
In addition to that, you could even “embed” that information into the function (or macro) name, for example naming the function something like make_u32_from_u16_hi_lo().
And, again, I believe that writing a reusable and well-tested helper function and invoking it when it’s needed is much better than spreading the code base with explicit bug-prone code where you can make mistakes with parentheses, with type casts, with bit-masking, etc.
LikeLike
Linus’ point is that when turned into an invocation, you don’t know which is low and which is high unless you check the function definition, and so can’t read what the code does without pointing your IDE at it. He’s completely right: every single one of these variations is a laid trap ready to be sprung by simply mixing up which goes first when you’re coding. The mistake would be invisible, impossible to test for and easy to make: the Bad Engineering trifecta.
LikeLike
Read my previous reply to commenter Steve.
Note that you can make even *more* mistakes by writing *explicit* code: you can make mistakes with the parentheses, the type casts, the bit-masking, etc.
If you do insist on the potential mistake of inverting the high word and low word when invoking the helper function (and it’s one potential error vs. several potential errors when writing explicit code!), you can name the function like make_u32_from_u16_hi_lo() or something similar.
LikeLike
In the macro case, consider using `0xFFFFu`
LikeLike
Thanks. Adding the ‘u’ suffix might be better for clarity. But when you do ‘(uint32_t)(low) & 0xFFFF’, the 0xFFFF is automatically converted to an unsigned int before applying the bitwise &.
Anyway, another point to consider for the explicit code (imagine fixing all the missing ‘u’ in explicit code spread throughout the code base!!) vs. reusing well-written-and-tested code in a macro/function, and invoke the tested code whenever it’s needed 🙂
LikeLike
John Ousterhout would call that a “shallow function” – Invoking a method isn’t much easier than just typing in the code of the method.
LikeLike
Did you read my article, and my replies to some other comments? Can you understand that invoking such a helper makes the code *safer* and *less bug prone* than writing _explicit_ code that can potentially have various bugs (bugs in type casting, bugs in bitmasking, etc.) that can be spread throughout the code base when _explicit_ bug-prone code is written?
To return the favor, I would define your comment a “shallow comment”.
LikeLike
LikeLike
1. I learned C first, then moved to C++. When I learned C++ (around 1995-1996), inline was a C++ keyword, but not a C keyword. After several years, I saw that inline was somehow “ported” from C++ to C, although with some differences. (For example, in C++ you can simply use “inline int f(…)” in header files, while in C it seems that corresponds to something like “static inline” instead.) Anyway, I’m glad that a *good* C++ feature was somehow “copied” to C, but that’s not the point of the topic.
2. I believe that a proper helper (be it a macro or inline function – I would prefer an inline function) that raises the semantic level of the code, and hides the details of well-tested code making it easily REUSABLE is much better than writing explicit bug-prone code. I mean, seriously?? “Let’s write explicit code! Who cares if we spread the code base with bugs! We’ll find them later with testing.” What kind of stupid, dangerous, unproductive and unprofessional attitude is that??!
And such a helper is much better also for readability.
For example, in Windows C header wingdi.h, there are some helper macros like:
#define RGB(r,g,b) ((COLORREF)(((BYTE)(r)|((WORD)((BYTE)(g))<<8))|(((DWORD)(BYTE)(b))<<16)))Or:
#define GetRValue(rgb) (LOBYTE(rgb))#define GetGValue(rgb) (LOBYTE(((WORD)(rgb)) >> 8))Etc.
I much more prefer using such helpers than writing explicit code with type casts, shifts, bitmasking, etc.! And I believe that using that kind of helpers (instead of writing “explicit code”) improves BOTH code correctness and READABILITY.
3. On the IDE point, I started programming on Windows with Visual C++ (I think the first version I used was Visual C++ 4.2 or VC++ 5, then I moved to the great VC++ 6, etc.), and I love high-quality IDEs. IDEs make you much more productive. For example: when squiggles for C++ were introduced in Visual Studio 2010, you could basically compile your C++ code and 99% of the times be sure that the code compiled successfully and there were no syntax errors (instead of spending compilation cycles and wasting precious time just to figure out that compilation failed because of syntax errors!). For the nitpickers: Yes, of course you have to test the code at runtime, thank you very much.
Quality IDEs help you in so many aspects, like browsing code bases, etc., that I won’t list here.
Quality IDEs are a GREAT tool for both hobbyists and professionals.
> Many Linux programmers are in it for the fun.
Well, using a good IDE can be very fun, and productive, too! 🙂
And, in addition to that, I already wrote that you can embed the information about parameter orders in the helper’s name, like adding “_hi_lo” or whatever to the helper’s name (I ALREADY wrote that other times, and I won’t repeat it anymore).
LikeLike