On Mon, Feb 12, 2024 at 12:24:39PM +0100, Lukas Wunner wrote: > Jonathan suggests adding cleanup.h support for x509_certificate structs. > cleanup.h is a newly introduced way to automatically free allocations at > end of scope: https://lwn.net/Articles/934679/ > > So add a DEFINE_FREE() clause for x509_certificate structs and use it in > x509_cert_parse() and x509_key_preparse(). These are the only functions > where scope-based x509_certificate allocation currently makes sense. > A third user will be introduced with the forthcoming SPDM library > (Security Protocol and Data Model) for PCI device authentication. > > Unlike most other DEFINE_FREE() clauses, this one checks for IS_ERR() > instead of NULL before calling x509_free_certificate() at end of scope. > That's because the "constructor" of x509_certificate structs, > x509_cert_parse(), returns a valid pointer or an ERR_PTR(), but never > NULL. > > I've compared the Assembler output before/after and they are identical, > save for the fact that gcc-12 always generates two return paths when > __cleanup() is used, one for the success case and one for the error case. > > In x509_cert_parse(), add a hint for the compiler that kzalloc() never > returns an ERR_PTR(). Otherwise the compiler adds a gratuitous IS_ERR() > check on return. > Introduce a handy assume() macro for this which can be > re-used elsewhere in the kernel to provide hints for the compiler. Shouldn't it be in a separate patch? ... > +#define assume(cond) do if(!(cond)) __builtin_unreachable(); while(0) Missing spaces? Missing braces (for the sake of robustness)? #define assume(cond) do { if (!(cond)) __builtin_unreachable(); } while (0) -- With Best Regards, Andy Shevchenko
