On Mon, Apr 29, 2024 at 04:03:07AM +0100, Matthew Wilcox wrote: > > diff --git a/include/linux/slab.h b/include/linux/slab.h > > index 4cc37ef22aae..5f5766219375 100644 > > --- a/include/linux/slab.h > > +++ b/include/linux/slab.h > > @@ -279,7 +279,7 @@ void kfree(const void *objp); > > void kfree_sensitive(const void *objp); > > size_t __ksize(const void *objp); > > > > -DEFINE_FREE(kfree, void *, if (_T) kfree(_T)) > > +DEFINE_FREE(kfree, void *, if (!IS_ERR_OR_NULL(_T)) kfree(_T)) > > Wait, why do we check 'if (_T)' at all? kfree() already handles NULL > pointers just fine. I wouldn't be averse to making it handle error > pointers either. > > > -DEFINE_FREE(kvfree, void *, if (_T) kvfree(_T)) > > +DEFINE_FREE(kvfree, void *, if (!IS_ERR_OR_NULL(_T)) kvfree(_T)) > > Ditto kvfree(). Fixing kfree() would fix both of these. I've always thought freeing pointers that have not been allocated is sloppy so I like that kfree() doesn't allow error pointers. We always catch it before it reaches production and that teaches people better habbits. Personally, I like how free_netdev() only accepts valid pointers. But I won't fight you on that if you want to change it. People have discussed this in the past, but no one has actually sent the patch. It would probably be merged. The __free() stuff is different because it's supposed to be transparent. Btw, I'm hoping we can officially declare small allocations as NOFAIL so then we can start doing allocations in the declaration block and remove the error checking and the cleanup. #define __ALLOC(p) p __free(kfree) = kmalloc(sizeof(*p), GFP_SMALL) #define __ZALLOC(p) p __free(kfree) = kzalloc(sizeof(*p), GFP_SMALL) struct foo *__ZALLOC(p); regards, dan carpenter
