On 2026-03-16T19:33:37+0100, Alejandro Colomar wrote: > Hi Kees, > > I just learnt about kalloc_objs() et al. from > . > > ptr = kmalloc_obj(*ptr); > ptr = kmalloc_objs(*ptr, n); > > This resembles a lot the macro we have in shadow-utils, malloc_T(), > which would be used as (to resemble the above): > > ptr = malloc_T(1, typeof(*ptr)); // But we'd really pass the type > ptr = malloc_T(n, typeof(*ptr)); // But we'd really pass the type > > But I've noticed some design mistakes that make it not as safe as it > seems. > > Default arguments > ~~~~~~~~~~~~~~~~~ > > I tend to think it's simpler to have a single API that works for both > 1 element and multiple elements. The special-casing of 1 element seems > unnecessary, and having a literal 1 works just fine. > > I think the combination of having the macros be variadic (for gfp) with > having two very similar APIs that differ in number of arguments, and > all those arguments being integer types, is prone to errors. Consider > the case where one would accidentally write > > ptr = kmalloc_obj(*ptr, n); // Bogus > instead of > ptr = kmalloc_objs(*ptr, n); > > The compiler wouldn't realize at all. That's a strong argument in > favour of having default arguments be required to be explicit, with an > empty argument: > > ptr = kmalloc_obj(*ptr,); > ptr = kmalloc_objs(*ptr, n,); > > I know you (and Linus too, FWIW) have previously claimed that it looks > weird to the eye. But I'm pretty sure you could get used to it. That's > certainly going to be safer. > > With mandatory empty arguments, the compiler would easily distinguish > mistakes like the one above. > > Type safety > ~~~~~~~~~~~ > > Apart from the issues with the above, the ability to pass a variable > instead of a type name is also a bad choice. In shadow-utils, we > require a type name, and a variable is rejected. We implement that with > the typeas() macro: > > #define typeas(T) typeof((T){0}) > > This macro works exactly like typeof(), but it requires that the input > is also a type. Passing a variable is a syntax error. We implement > malloc_T() with it: > > // malloc_T - malloc type-safe > #define malloc_T_(n, T) \ > ({ \ > (typeas(T) *){reallocarray(n, sizeof(T))}; \ I meant the following: (typeas(T) *){reallocarray(NULL, n, sizeof(T))}; I had the issue while pasting and adapting for simplicity. The original code is correct, of course. > }) > > which is used as (taking some arbitrary examples from shadow-utils): > > lp = xmalloc_T(1, struct link_name); > targs = xmalloc_T(n_args + 3, char *); > > Some reasons for passing a type name instead of a variable are: > > - It allows grepping for all allocations of a given type. > - It adds readability. It's similar to declaring variables with some > explicit type, vs. using 'auto' (__auto_type) everywhere. > > But there's also a safety aspect. Consider we want to allocate an array > of 42 ints. And consider the programmer accidentally swaps arguments. > > int *p = malloc_T(int, 42); // syntax error > int *p = malloc_T(42, int); > vs > int *p = kmalloc_objs(*p, 42); > int *p = kmalloc_objs(42, *p); // Bogus > > The latter is dereferencing an uninitialized pointer. If for some > reason the pointer had a value before this call, you'd be allocating as > many elements as *p says, which would be bogus, and since typeof(42) is > the same as typeof(*p), the return type would be valid, so this would > still compile. > > > Have a lovely night! > Alex > > -- > --