Shaochun Chen <cscnull@xxxxxxxxx> wrote: [ CC ing Tom, as he added ->start() ] > 1) if netlink_dump_start start fail, the memory of c->data will leak. > so free manually after netlink_dump_start return error. > > 2) In netlink_dump_start, ignore the return of netlink_dump. > Because if cb_running is set to true, cb->dump will be call in anyway. > so if netlink_dump_start start successfully, just return -EINTR. This now requires ->start() to not return -EINVAL, else we won't release resources either, this seems fragile to me. I can only think of three ways to address this: 1. Kill ->start() and force all users to defer state allocation to first ->dump() invocation, so existing ->done() can be used to release resources. OR 2. add ->stop() and have core always pair it with ->start(), no matter if dump() got called or not, then convert all places that provide .start to use .stop, not .done. OR 3. change meaning of ->done() so its always called once ->start() was invoked (and returned 0), this requires audit of all places that provide .done to make sure they won't trip. 3) seems to be what Tom intended when he added .start, so probably best to investigate that first. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
