On Mon, Dec 27, 2021 at 11:54:10PM +0800, Alex Elder wrote: > I think this is a good change, but I would like you to improve > the description, and fix some different bugs introduced by your > change. > > What you are specifically doing is checking for a null return > from devm_kcalloc() in gb_generate_enum_strings(), and are > returning the NULL pointer if that occurs. That means you > need to update all the callers of gb_generate_enum_strings() > to also handle a possible null return value. > > The fix does a good thing, and your description is correct > about what you are fixing. But it should supply more > complete context for the change. Thanks for your advice, I will correct my description in next version. But I still have some question about the devm_kzalloc(). > You can't simply return here. If you look a bit above this, > where the call to allocate a control structure is done, you > see that a NULL return there jumps to the "error" label, so > any already allocated and initialized control widgets get > cleaned up before returning. Actually, I have already thought of whether it needs to free after the devm_kzalloc(). As we can find in the gbaudio_tplg_create_widget(), the widget_kctls is allocated by devm_kzalloc(), but isn't released when gbaudio_tplg_create_wcontrol() fails and goto error. And I check of the comment of the devm_kmalloc() in `drivers/base/devres.c`, because devm_kzalloc() returns devm_kmalloc(). And it says that "Memory allocated with this function is automatically freed on driver detach." So there is no need to free the memory manually. Is that right? And I am sorry again because of the lack of the above explanation in my commit message. I will also add to my new commit. Thanks, Jiang
