lkml.org 
[lkml]   [2022]   [Nov]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH net-next 9/9] net: ipa: use a bitmap for enabled endpoints
From
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);

\
 
 \ /
  Last update: 2022-11-02 13:45    [W:0.895 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site