Hi Daniel, On Thu, 2020-07-23 at 00:22 +0200, daniel@xxxxxxxx wrote: [...] > Yeah the drmm_ versions of these need to check that the ->cleanup hook is > NULL. > > Also there's not actually a double-free, since drm_foo_cleanup removes it > from the lists, which means drm_mode_config_cleanup won't even see it. But > if the driver has some additional code in ->cleanup that won't ever run, > so probably still a bug. > > I also think that the drmm_foo_ wrappers should also do the allocation > (and upcasting) kinda like drmm_dev_alloc(). Otherwise we're still stuck > with tons of boilerplate. Ok, I'll try this: drmm_encoder_init() variant can verify that the passed drm_encoder_funcs::destroy hook is NULL. drmm_simple_encoder_init() can just provide empty drm_encoder_funcs internally. > For now I think it's ok if drivers that switch to drmm_ just copypaste, > until we're sure this is the right thing to do. And then maybe also roll > these out for all objects that stay for the entire lifetime of drm_device > (plane, crtc, encoder, plus variants). Just to make sure we're consistent > across all of them. Thank you for clarifying, I wasn't sure this was the goal. I've started with this function mostly because this is the most used one in imx-drm and the only one where I didn't have to deal with va_args boilerplate. regards Philipp _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
