Hacker News new | ask | show | jobs
by perihelions 2144 days ago
"(current_euid() is a macro but it's written to not be a permitted lvalue)"

I'm not an expert at C. I followed up on this kernel macro out of curiosity, and it was a confusing learning experience because it turns out the forbidden assignment

    ({ x; }) = y;
is silently permitted by GCC (for example, with -Wall --std={c99,c11,c18}), and does actually assign x=y. Even though that's expressly prohibited by the C standard (-Wpedantic).

I assume this is old news to C programmers, but its insidiousness surprised me.

4 comments

Example #5984 of why I don't like the kernel's convention of masquerading macros as function by making them lowercase. I wasted so much time deciphering weird compile errors or strange behaviour only to finally realize that one of the function calls in the offending code was actually a macro in disguise.

It's especially bad when some kernel macros, such as wait_event, don't even behave like a function would (evaluating the parameter repeatedly).

One more thing Rust got right by suffixing macros with a mandatory !.

The best one is "current" -- which is a macro that looks like a variable but becomes a function call and thus if you ever want to use the variable name "current" you will get build errors. :D
Huh, I assumed (just as you did) that this would obviously not work -- but you're right that GCC ignores this and allows the assignment anyway.

However it turns out that you still get a build error, and even the more explicit versions also give you a error:

  kernel/cred.c:763:17: error: assignment of member ‘euid’ in read-only object
    763 |  current_euid() = GLOBAL_ROOT_UID;
        |                 ^
  kernel/cred.c:764:23: error: assignment of member ‘euid’ in read-only object
    764 |  current_cred()->euid = GLOBAL_ROOT_UID;
        |                       ^
  kernel/cred.c:765:22: error: assignment of member ‘euid’ in read-only object
    765 |  current->cred->euid = GLOBAL_ROOT_UID;
        |                      ^
So it is blocked but not for the reason I thought. current_cred() returns a const pointer and all of the cred pointers in task_struct are also const. So you'd need to do something more like:

  ((struct cred *)current_cred())->euid = GLOBAL_ROOT_UID;
Which is well beyond "eyebrow-raising" territory.
The C standard cannot expressly prohibit anything about a feature that isn't part of the C standard.
What you quoted:

    ({ x; }) = y;
isn't ISO C syntax; it's a GNU extension.

-Wpedantic diagnoses ISO C syntax errors, even if they are GNU extensions.