Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754353AbcDKMLc (ORCPT ); Mon, 11 Apr 2016 08:11:32 -0400 Received: from mga04.intel.com ([192.55.52.120]:44337 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752890AbcDKMLb (ORCPT ); Mon, 11 Apr 2016 08:11:31 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,462,1455004800"; d="asc'?scan'208";a="952483819" From: Felipe Balbi To: "Du\, Changbin" Cc: "gregkh\@linuxfoundation.org" , "linux-usb\@vger.kernel.org" , "linux-kernel\@vger.kernel.org" Subject: RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit In-Reply-To: <0C18FE92A7765D4EB9EE5D38D86A563A05CFE28D@shsmsx102.ccr.corp.intel.com> References: <1460108522-31664-1-git-send-email-changbin.du@intel.com> <87bn5gpo1v.fsf@intel.com> <0C18FE92A7765D4EB9EE5D38D86A563A05CFE203@shsmsx102.ccr.corp.intel.com> <87shyso0tp.fsf@intel.com> <0C18FE92A7765D4EB9EE5D38D86A563A05CFE28D@shsmsx102.ccr.corp.intel.com> User-Agent: Notmuch/0.21+96~g9bbc54b (http://notmuchmail.org) Emacs/25.0.90.3 (x86_64-pc-linux-gnu) Date: Mon, 11 Apr 2016 15:09:34 +0300 Message-ID: <87mvp0ny81.fsf@intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2187 Lines: 66 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, "Du, Changbin" writes: >>=20 >> >> > + dwc->regset =3D 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 h= abit to >> > Avoid wild pointers. Just like the dwc->root =3D NULL. >>=20 >> there won't be any wild pointers here, we'll free struct dwc3 *dwc itsel= f. >>=20 >> -- >> 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. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXC5P+AAoJEIaOsuA1yqRE6+wQALYvMTZimNqQD0ZBtxW0YIuo NC9CldZWQwvEvzsi/nLx21R9QOdB8tvSmcYQsbzFecGsRWZAexiQQXdIiGXqyNFp 3fa1R+XVMI4RDb564b7Tna41SpTIb3Wwjr2MThy2KGJCC0fN2xy8kDtynr/3kLTd hhA0ElofPldbOKN3hN8+AuiaHmAWQlPn4H+Fll/dRd8rFG1LQg/kQ/RPgU3lzHTm Alz6AKMUFg7BUnBBse40t48dAA1rb1B8elfmYtsZU1VEKvg2/xwLHIoFrM4L3jGQ vBQRVtxe3jazD0dJEGuOp0ZMVUneQ5S3MBWICGEC1deyr/gOgHJRcUo5+v9xmgCK zUpZvT4ncXf4YnxyTVUr0NmnSXE5BEda52Zzr08YXs6ILPLxGFYaJ5IQzsY7aFIo AoOFmHOp8Cxm+qQU/NwtzoBqBJq2Ctl6gCaBVvBoEMcAOXuQrMwO1XJE7vwQum5U 0wSbpBkEjP6TvaPmnwqEcQ+lDza5gSWsY10GsJKVlfLM9jRlhDeImD9XT40ni6hB k4ofjWG1EwtFes7UCxTlCfBLQjMPjNTYfMsdqLuvmTs8Dw/hLNB//fNe9yWk9OnX AULQJLGT/0MRclEcM9cVbXY/L5fcpsLxVc5OD5DdjT6UzYNHtepP23hAK9xUqfpP m2gaeGZAoEC9ZmzwQrYQ =1KyX -----END PGP SIGNATURE----- --=-=-=--