Messages in this thread | | | Date | Wed, 2 Nov 2022 07:44:35 -0500 | Subject | Re: [PATCH net-next 9/9] net: ipa: use a bitmap for enabled endpoints | From | Alex Elder <> |
| |
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);
| |