[EXT] Re: [PATCH] USB: gadget: Add ID numbers to configfs-gadget driver names

Frank Li frank.li at nxp.com
Fri Dec 16 03:15:55 AEDT 2022


]
> >
> > Move all free into gadget_info_attr_release(), just before kfree(gi)
> > Driver.name and gi create at the same place,
> > Free should be the same place also.
> >
> 
> Thanks a lot for the quick review comment.
> 
> As per my observation through the test, on the first mount, the
> virtual-media the gadgets_make() is called, then later, when unmount,
> the gadgets_drop() is called and followed by gadget_info_attr_release().
> 
> The gadget_info_attr_release() is registered as .release() of
> gadget_root_type for the gi->group via the call
> "config_group_init_type_name(&gi->group, name, &gadget_root_type);"
> 
> In general, the .release() will be called only for the group. There is
> nothing to guarantee that the group will always be registered, ie:
> incase without the call to "config_group_init_type_name(&gi->group,
> name, &gadget_root_type);"
> 
> In this patch, what is added is an ida number to be used to make up the
> composite driver name. This is done in gadgets_make() so we'd like to
> add the cleanup code to gadgets_drop() as they are registered together
> in the same place and would be a little easier to read than adding them
> to _release() as the code below:
> 
>      static struct configfs_group_operations gadgets_ops = {
>          .make_group = &gadgets_make,
>          .drop_item = &gadgets_drop,
>      };
> 
> Anyway, we still doubt that there might be something that we have missed
> so please let me know the reason why putting cleanup codes to _release()
> would be a better solution.

Let's simply the logic

Gadget_make()
{
        gi=kazalloc();
        gi->composite.gadget_driver.driver.name = kasprintf();
        gi->compositr.gadget_drver.function = kstrdup();
}

this is only place to create gi and driver.name. 
no other place to set driver.name,  so I think gi take ownship of driver.name. 

so we only track when gi free,   just free driver.name before gi free. 

Related malloc and free should be pair together. 

Gadget_info_attr_release()
{
	Kfree(gi->composite.gadget_driver.function)
	Kfree(gi->composiste.gadget_driver.driver.name);
	Kfree(gi);
}

Gadget_driver.driver.name should be similar as gadget_driver.function

If driver.name is freed in gadget_drop(), gi can not actually be reused again.



> 
> Thank you and best regards,
> - Chanh
> 
> >
> >>>>        config_item_put(item);
> >>>>    }
> >>>


More information about the openbmc mailing list