superio: Use a bus_child_deleted method to free ivars for children
ClosedPublic
Actions

Authored by jhb on Oct 31 2024, 8:36 PM.

Details

Reviewers

imp

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

Summary

Previously these ivars were leaked.

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 60304
Build 57188: arc lint + arc unit

Owners added a reviewer: Restricted Owners Package.Oct 31 2024, 8:36 PM

jhb requested review of this revision.Oct 31 2024, 8:36 PM

(more a comment in passing than a request for change)

sys/dev/superio/superio.c
780	After seeing a bunch of these, I kinda wonder if there's some way we can refactor this into newbus. Though given that, at best, the ivar might have a resource list and there's no 'baseclass' requirement, that might be hard.

This revision is now accepted and ready to land.Oct 31 2024, 9:04 PM

jhb added inline comments.Nov 1 2024, 2:04 PM

sys/dev/superio/superio.c
780	Maybe the actual free could be pulled up into new-bus similar how softc allocation is managed, but the resource_list bit means you'd end up with a child_deleted method anyway, so I'm not sure it buys much. There are a couple of related things it might be nice to fix to be more consistent in this area: Bus drivers that allocate ivars should probably always be using a custom bus_add_child method where these are initialized. Right now some buses setup ivars in the caller of BUS_ADD_CHILD. OTOH, I think I would actually prefer to remove BUS_ADD_CHILD and switch to a BUS_CHILD_ADDED that is always called from device_add_child(). I think the latter approach would be more robust, and it would also be a bit more obvious that bus_child_deleted should be paired with bus_child_added. bus_child_deleted methods should probably be using resource_list_purge() instead of resource_list_free(). Arguably resource_list_free() should just be resource_list_purge()?