Re: [PATCH] ath12k: fix NULL pointer dereference in rhash table destroy

[Jose Ignacio Tornos Martinez]

Hello Jeff,

Thank you for the feedback.

> My preference would be to fix the logic so that the deinit is symmetric with
> init, and if any stage of init fails, it should unwind whatever init occurred
> up to that point. So this patch seems to be a bandage instead of an engineered
> fix.
>
> But I'll let the Qualcomm engineering team give their opinion.
I understand the concern about symmetry.
The NULL check approach is intentional here. The ath12k driver has complex
multi-stage initialization, with many potential failure points.
The init/deinit paths are symmetric - destroy is called from the same
cleanup paths as init.
The issue is that init can fail partway through, leaving some components
uninitialized.
The rhashtable pointer itself serves as the initialization state indicator -
it's NULL before init and non-NULL after. Checking it directly is simpler
than adding separate state flags that must be kept in sync.
That said, I'm open to alternative approaches if you see a cleaner way to
handle partial initialization states across the driver's initialization
sequence. Happy to revise if there's a better solution I'm missing.

> not a fan of this pattern.
> if we go with a bandaid solution then i'd want to use guard(mutex) so that
> there isn't a need for a cleanup goto -- we could just return like the patch below
Agreed, guard(mutex) is cleaner. I'll incorporate this in v2 once we align
on the overall approach.

Thanks

Best regards
José Ignacio