On 11/1/22 11:34 PM, Jakub Kicinski wrote:
On Sat, 29 Oct 2022 19:18:28 -0500 Alex Elder wrote:
/* Set up the defined endpoint bitmap */
ipa->defined = bitmap_zalloc(ipa->endpoint_count, GFP_KERNEL);
ipa->set_up = bitmap_zalloc(ipa->endpoint_count, GFP_KERNEL);
+ ipa->enabled = bitmap_zalloc(ipa->endpoint_count, GFP_KERNEL);
if (!ipa->defined || !ipa->set_up) {
This condition should now check if ipa->enabled
You're right.
And the error handling patch needs to free it, in case it was something
else that didn't get allocated?
See below, but in any case, I'll make sure this is right.
Frankly I have gotten mass-NULL-checks wrong more than once myself so
I'd steer clear of those, they are strangely error prone.
I don't typically do it, and generally don't like it,
but I think I was trying to make it look cleaner
somehow. I'll check every one in separately in v2.
dev_err(dev, "unable to allocate endpoint bitmaps\n");
this error message should not be here (patch 5 adds it I think)
memory allocation failures produce a splat, no need to print errors
At the end of the series, the structure of this
error handling changes, and I think this is
correct. But now that you point this out I
think the way this evolves in the series could
use some improvement so I'll take another look
at it and hopefully make it better.
I don't normally report anything on allocation
failures for that reason; thanks for pointing
this out.
I really appreciate your feedback. I'll send
out version 2 today.
-Alex
+ bitmap_free(ipa->set_up);
+ ipa->set_up = NULL;
bitmap_free(ipa->defined);
