On Thu, Nov 30, 2023 at 04:11:18PM -0500, Rae Moar wrote: > > + stream = alloc_string_stream(GFP_KERNEL); > > + if (IS_ERR_OR_NULL(stream)) > > In response to Dan Carpenter's comment from the last version, I see > the benefits of changing IS_ERR_OR_NULL() to IS_ERR() instead because > "stream" will not be NULL. This would then also be the same as the > check in kunit_alloc_string_stream. > > However, I also see the benefit of checking for NULL just in case anyways. > Returning NULL in alloc_string_stream() is a bug. Checking for NULL is a work around for bugs. There are basically two times where it can be valid to work around bugs like this instead of fixing them. 1) When the function is implemented by over 10 different driver authors. In that case you can guarantee that at least one of them is going to do the wrong thing. There are between 2-5 places which do this in the kernel. 2) If it's a API that used to return NULL and it's changed to returning error pointers. I've never seen anyone do this, but I've proposed it as a solution to make backporting easier. regards, dan carpenter
