On Fri, Mar 01, 2024 at 06:10:33PM +0800, Herbert Xu wrote: > On Tue, Feb 20, 2024 at 04:10:39PM +0100, Lukas Wunner wrote: > > > > 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. > > Would it be possible to move the use of assume into the kzalloc > declaration instead? Perhaps by turning it into a static inline > wrapper that does the "assume"? > > Otherwise as time goes on we'll have a proliferation of these > "assume"s all over the place. I've tried moving the assume(!IS_ERR()) to kmalloc() (which already is a static inline), but that increased total vmlinux size by 448 bytes. I was expecting pushback due to the size increase, hence kept the assume() local to x509_cert_parse(). There's a coccinelle rule which warns if an IS_ERR() check is performed on a kmalloc'ed pointer (scripts/coccinelle/null/eno.cocci), hence there don't seem to be any offenders left in the tree which use this antipattern and adding assume(!IS_ERR()) to kmalloc() doesn't have any positive effect beyond avoiding the single unnecessary check in x509_cert_parse(). If you don't like the assume(!IS_ERR(cert)) in x509_cert_parse(), I can respin the patch to drop it. The unnecessary check which it avoids only occurs in the error path. If the certificate can be parsed without error, there's no unnecessary check. It still triggered my OCD when scrutinizing the disassembled code and sufficiently annoyed me that I wanted to get rid of it, but in reality it's not such a big deal. Thanks, Lukas
