From: Felipe Balbi <balbi@kernel.org>
To: "Du\, Changbin" <changbin.du@intel.com>
Cc: "gregkh\@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"linux-usb\@vger.kernel.org" <linux-usb@vger.kernel.org>,
"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
Date: Mon, 11 Apr 2016 15:09:34 +0300 [thread overview]
Message-ID: <87mvp0ny81.fsf@intel.com> (raw)
In-Reply-To: <0C18FE92A7765D4EB9EE5D38D86A563A05CFE28D@shsmsx102.ccr.corp.intel.com>
[-- Attachment #1: Type: text/plain, Size: 1219 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
next prev parent reply other threads:[~2016-04-11 12:11 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-08 9:42 [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit changbin.du
2016-04-11 8:06 ` Felipe Balbi
2016-04-11 11:09 ` Du, Changbin
2016-04-11 11:13 ` Felipe Balbi
2016-04-11 11:38 ` Du, Changbin
2016-04-11 12:09 ` Felipe Balbi [this message]
2016-04-12 2:10 ` Du, Changbin
2016-04-12 6:18 ` Felipe Balbi
2016-04-12 6:52 ` Du, Changbin
2016-04-12 8:06 ` Felipe Balbi
2016-04-12 8:14 ` [PATCH v2] usb: dwc3: fix memory leak of dwc->regset changbin.du
2016-04-12 8:26 ` Felipe Balbi
2016-04-12 8:24 ` [PATCH v3] " changbin.du
2016-04-12 10:09 ` Felipe Balbi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87mvp0ny81.fsf@intel.com \
--to=balbi@kernel.org \
--cc=changbin.du@intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.