All of lore.kernel.org
 help / color / mirror / Atom feed
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 --]

  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.