Hi, "Du, Changbin" <changbin.du@xxxxxxxxx> 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
Attachment:
signature.asc
Description: PGP signature
