Messages in this thread | | | From | Felipe Balbi <> | Subject | RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit | Date | Tue, 12 Apr 2016 09:18:17 +0300 |
| |
Hi Changbin,
"Du, Changbin" <changbin.du@intel.com> writes: >> Hi, >> >> "Du, Changbin" <changbin.du@intel.com> writes: >> >> >> >> >> > + dwc->regset = NULL; >> >> >> >> >> >> setting regset to NULL is unnecessary. We only call >> dwc3_debugfs_exit() >> >> >> when removing the driver. >> >> >> >> >> >> -- >> >> >> Balbi >> >> > I'd like keep this line even it is unnecessary, because It is a good habit to >> >> > Avoid wild pointers. Just like the dwc->root = NULL. >> >> >> >> there won't be any wild pointers here, we'll free struct dwc3 *dwc itself. >> >> >> >> -- >> >> Balbi >> > I agree the dwc will be freed in current code. But the 'free' logical is out >> > of the debugfs code. They should be treat as some logical independent. >> Per >> > this point, I still think set pointer to null is not bad. For example, if dwc3 >> core >> > code invoke dwc3_debugfs_exit twice by mistake(just an example case, >> not >> > really), then no crash/impact for the second call. >> >> the second call should crash because it's clearly wrong ;-) If dwc3 ever >> calls dwc3_debugfs_exit() twice, it really deserves to crash. It's >> something so wrong that we want the verbosity and urgency of a kernel >> oops to make sure we fix it ASAP. >> >> If, however, we set it to null, it might be years before we notice >> anything's wrong. >> >> -- >> Balbi > > Hmm, I agree from this point. I will combine this patch with other two patches > (due to their dependency). And I'd like remove the 'dwc->root=NULL' as well,
you are creating a dependency that doesn't exist. Please stop that. You should have two separate branches based on v4.6-rc3 (or, if you prefer, one based on my testing/fixes and another based on my testing/next). On one branch you have *only* $subject and you fix *all* the memory leaks. On the other branch you have the other two patches.
Ignore the fact that we might have a conflict, that's for git (and maintainers) to handle when they happen.
Again, don't create dependencies between fixes for the -rc cycle and changes for the next merge window.
> Is it ok for you?
yeah, please remove root = NULL as that's completely unnecessary, but split these patches in separate branches and fix all memory leaks.
-- balbi [unhandled content-type:application/pgp-signature]
| |