On Fri, 14 Mar 2025 at 17:15, Kees Cook <kees@xxxxxxxxxx> wrote: > > Introduce type-aware kmalloc-family helpers to replace the common > idioms for single, array, and flexible object allocations: > > kmalloc_obj(ptr, gfp); > [ ... ] Honestly, I absolutely hate this. Don't do this. It's a huge mistake. Yes, it's a really easy and convenient interface to use. And it's a ABSOLUTELY HORRENDOUSLY BAD interface to actually then *maintain*. Why? Because it's simply visually and syntactically entirely wrong. It's much much too easy to miss that there's an assignment there, because the assignment is hidden inside that macro that visually looks like a function call. So when you scan the code, the data flow becomes very hard to see. So no. A hard NAK on this. It's wrong, it's bad, and it's crap. Maintaining code is AT LEAST as important as writing it, and arguably much more so. And making code and data flow visually clear is important, and this is actively detrimental to that. So I understand why you want to do this, but no, this is absolutely not the way to do it. It needs at a minimum some way to make it very very visually clear that this is an assignment to 'ptr', and honestly, I do not see how to do that cleanly. Alternatively, this might be acceptable if the syntax makes mistakes much harder to do. So for example, if it wasn't just an assignment, but also declared the 'ptr' variable, maybe it would become much safer simply because it would make the compiler warn about mis-uses. Using visual cues (something that makes it look like it's not a regular function call) might also help. The traditional C way is obviously to use ALL_CAPS() names, which is how we visually show "this is a macro that doesn't necessarily work like the normal stuff". Some variation of that might help the fundamental issue with your horrendous thing. But something very serious needs to be done before this is acceptable. Because no, the advantage of writing kmalloc_obj(ptr, gfp); instead of ptr = kmalloc(sizeof(*ptr), gfp); is absolutely NOT worth the horrendous disadvantages of that disgusting and wrong syntax. You saved a handful of characters, and made the code faster to write, at the cost of making the result be much worse. My suggestion would be to look at some bigger pattern, maybe including that declaration. To take a real example matching that kind of pattern, we have struct mod_unload_taint *mod_taint; ... mod_taint = kmalloc(sizeof(*mod_taint), GFP_KERNEL); and maybe those *two* lines could be combined into something saner like ALLOC(mod_taint, struct mod_unload_taint, GFP_KERNEL); which would stand out visually (which is admittedly very different from being "pretty" ;), and would also save you some typing because it also gets rid of the declaration. We allow declarations in the middle of code these days because we needed it for the guarding macros, so this would be a new kind of interface that dos that. And no, I'm not married to that disgusting "ALLOC()" thing. I'm throwing it out not as TheSOlution(tm), but as a "this interface absolutely needs clarity and must stand out syntactically both to humans and the compiler". Linus