From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751195AbcBLO5j (ORCPT ); Fri, 12 Feb 2016 09:57:39 -0500 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:51904 "EHLO metis.ext.4.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750802AbcBLO5h (ORCPT ); Fri, 12 Feb 2016 09:57:37 -0500 From: Markus Pargmann To: weiyj_lk@163.com Cc: Jens Axboe , Wei Yongjun , nbd-general@lists.sourceforge.net, linux-kernel@vger.kernel.org Subject: Re: [PATCH] nbd: Fix return value check for debugfs_create_*() Date: Fri, 12 Feb 2016 15:57:23 +0100 Message-ID: <10723389.IcVAbx7fZ7@adelgunde> User-Agent: KMail/4.14.1 (Linux/4.3.0-0.bpo.1-amd64; KDE/4.14.2; x86_64; ; ) In-Reply-To: <1454680101-17853-1-git-send-email-weiyj_lk@163.com> References: <1454680101-17853-1-git-send-email-weiyj_lk@163.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart2571402.bUZ0QDv39V"; micalg="pgp-sha256"; protocol="application/pgp-signature" X-SA-Exim-Connect-IP: 2001:67c:670:100:a61f:72ff:fe68:75ba X-SA-Exim-Mail-From: mpa@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --nextPart2571402.bUZ0QDv39V Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="us-ascii" Hi, On Friday, February 05, 2016 09:48:21 PM weiyj_lk@163.com wrote: > From: Wei Yongjun >=20 > In case of error, the function debugfs_create_*() returns NULL > pointer not ERR_PTR() if debugfs is enabled. The IS_ERR() test > in the return value check should be replaced with NULL test. I tried to fix this already but it didn't reach 4.5. It is queued for 4.6. Please have a look at =09http://git.pengutronix.de/?p=3Dmpa/linux-nbd.git;a=3Dcommitdiff;h=3D= 27ea43fe2a32f63bb6f442dafc2133232b8af4a6;hp=3D92e963f50fc74041b5e9e744c= 330dca48e04f08d whether this works for you as well. The difference is that I removed al= l error checks for the debugfs files. The code looks nicer but of course there are no error messages if it went wrong. Thanks, Markus >=20 > Signed-off-by: Wei Yongjun > --- > drivers/block/nbd.c | 50 ++++++++++++++++++++++++-------------------= =2D------ > 1 file changed, 24 insertions(+), 26 deletions(-) >=20 > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index e4c5cc1..02bc697 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -895,49 +895,47 @@ static int nbd_dev_dbg_init(struct nbd_device *= nbd) > =09struct dentry *f; > =20 > =09dir =3D debugfs_create_dir(nbd_name(nbd), nbd_dbg_dir); > -=09if (IS_ERR_OR_NULL(dir)) { > -=09=09dev_err(nbd_to_dev(nbd), "Failed to create debugfs dir for '%s= ' (%ld)\n", > -=09=09=09nbd_name(nbd), PTR_ERR(dir)); > -=09=09return PTR_ERR(dir); > +=09if (!dir) { > +=09=09dev_err(nbd_to_dev(nbd), "Failed to create debugfs dir for '%s= '\n", > +=09=09=09nbd_name(nbd)); > +=09=09return -ENOMEM; > =09} > =09nbd->dbg_dir =3D dir; > =20 > =09f =3D debugfs_create_file("tasks", 0444, dir, nbd, &nbd_dbg_tasks= _ops); > -=09if (IS_ERR_OR_NULL(f)) { > -=09=09dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'tasks= ', %ld\n", > -=09=09=09PTR_ERR(f)); > -=09=09return PTR_ERR(f); > +=09if (!f) { > +=09=09dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'tasks= '\n"); > +=09=09goto fail; > =09} > =20 > =09f =3D debugfs_create_u64("size_bytes", 0444, dir, &nbd->bytesize)= ; > -=09if (IS_ERR_OR_NULL(f)) { > -=09=09dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'size_= bytes', %ld\n", > -=09=09=09PTR_ERR(f)); > -=09=09return PTR_ERR(f); > +=09if (!f) { > +=09=09dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'size_= bytes'\n"); > +=09=09goto fail; > =09} > =20 > =09f =3D debugfs_create_u32("timeout", 0444, dir, &nbd->xmit_timeout= ); > -=09if (IS_ERR_OR_NULL(f)) { > -=09=09dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'timeo= ut', %ld\n", > -=09=09=09PTR_ERR(f)); > -=09=09return PTR_ERR(f); > +=09if (!f) { > +=09=09dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'timeo= ut'\n"); > +=09=09goto fail; > =09} > =20 > =09f =3D debugfs_create_u32("blocksize", 0444, dir, &nbd->blksize); > -=09if (IS_ERR_OR_NULL(f)) { > -=09=09dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'block= size', %ld\n", > -=09=09=09PTR_ERR(f)); > -=09=09return PTR_ERR(f); > +=09if (!f) { > +=09=09dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'block= size'\n"); > +=09=09goto fail; > =09} > =20 > =09f =3D debugfs_create_file("flags", 0444, dir, &nbd, &nbd_dbg_flag= s_ops); > -=09if (IS_ERR_OR_NULL(f)) { > -=09=09dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'flags= ', %ld\n", > -=09=09=09PTR_ERR(f)); > -=09=09return PTR_ERR(f); > +=09if (!f) { > +=09=09dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'flags= '\n"); > +=09=09goto fail; > =09} > =20 > =09return 0; > +fail: > +=09debugfs_remove_recursive(nbd->dbg_dir); > +=09return -ENOMEM; > } > =20 > static void nbd_dev_dbg_close(struct nbd_device *nbd) > @@ -950,8 +948,8 @@ static int nbd_dbg_init(void) > =09struct dentry *dbg_dir; > =20 > =09dbg_dir =3D debugfs_create_dir("nbd", NULL); > -=09if (IS_ERR(dbg_dir)) > -=09=09return PTR_ERR(dbg_dir); > +=09if (!dbg_dir) > +=09=09return -ENOMEM; > =20 > =09nbd_dbg_dir =3D dbg_dir; > =20 >=20 >=20 =2D-=20 Pengutronix e.K. | = | Industrial Linux Solutions | http://www.pengutronix.de/= | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 = | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-555= 5 | --nextPart2571402.bUZ0QDv39V Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part. Content-Transfer-Encoding: 7Bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJWvfLTAAoJEEpcgKtcEGQQiQcQAIXGyiAy5MDS8buYhgHBUypm 645FA9OBei5RFoDjQqIpm4aTBvxmRWyEZXqRa1CJQoHxdI/sbuSiQvh52ZM2j0bM GmzpHBIHSFbhHTccFDmnAkdhQxe8neBiBTDiAi7GKFBOxJQKaZJMwblMWbT+SrBn z3TfpWEis6Zu5v7Y156+FZnGjze9xdXqAKQAVwvKwhouUII3DAhrLSirgkBlsfMx LQa2wUnyHnwADXgMNvt+syVTxba1/JfGc9zch7BUMs0EeTVoWoHS875YUBI9t+WU EAv0uAJrv2mdxgQE8F9LH4gRTRSoX9phKwt0jbjy8JPWDt+FYF7YxPJjwXzc0GEG 2zzWth31W56LdBoeYGDn4pATQTTGrUf1OtHxEIldLuxvIiekfxXVLSQNd6mVrmqe 4ATlNf4nWls1SwB4499zrT9zL2M4BcX5ZfJQGyNMhKnt3nKsPKZHOMAppapoT+iz JUx7c+bMteI5pTPLGII8DuEmhdUV0etzbnx41W8EEesZK566VUMwiAWzHeVow9yv uKaSusPutZugJTG6QjJOYh5urhnfzAY6UlJ2vCYLFP7ic1UsFulKNC6Sz/lWsUOo ZjPc4gAxhYRpv9M9m1AskUFg/aYhsPNrYTuql4C48Z/xQsscFrOnpvC3NGlrQ8Bg MMCDt78070SsnjMx8GdM =bxjf -----END PGP SIGNATURE----- --nextPart2571402.bUZ0QDv39V--