linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Markus Elfring <Markus.Elfring@web.de>
To: Namjae Jeon <namjae.jeon@samsung.com>, linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org,
	"Christoph Hellwig" <hch@lst.de>,
	"Daniel Wagner" <dwagner@suse.de>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Nikolay Borisov" <nborisov@suse.com>,
	"Sungjong Seo" <sj1557.seo@samsung.com>,
	"Valdis Klētnieks" <valdis.kletnieks@vt.edu>,
	linkinjeon@gmail.com
Subject: Re: [PATCH v4 04/13] exfat: add directory operations
Date: Thu, 21 Nov 2019 14:07:15 +0100	[thread overview]
Message-ID: <498a958f-9066-09c6-7240-114234965c1a@web.de> (raw)
In-Reply-To: <20191121052618.31117-5-namjae.jeon@samsung.com>

…
> +++ b/fs/exfat/dir.c
> +static int exfat_readdir(struct inode *inode, struct exfat_dir_entry *dir_entry)
> +{
> +			if (!ep) {
> +				ret = -EIO;
> +				goto free_clu;
> +			}

How do you think about to move a bit of common exception handling code
(at similar places)?

+			if (!ep)
+				goto e_io;


…
> +free_clu:
> +	kfree(clu);
> +	return ret;

+
+e_io:
+	ret = -EIO;
+	goto free_clu;

> +}
> +static void exfat_set_entry_type(struct exfat_dentry *ep, unsigned int type)
> +{
> +	if (type == TYPE_UNUSED) {
> +		ep->type = EXFAT_UNUSED;
> +	} else if (type == TYPE_DELETED) {
> +		ep->type &= EXFAT_DELETE;

The other assignments are working without the ampersand.
Thus I would find its usage suspicious at this place.


…
> +int update_dir_chksum(struct inode *inode, struct exfat_chain *p_dir,
> +		int entry)
> +{
> +out_unlock:
> +	brelse(fbh);

Can the label “release_fbh” be more helpful?


…
> +struct exfat_entry_set_cache *exfat_get_dentry_set(struct super_block *sb,
> +		struct exfat_chain *p_dir, int entry, unsigned int type,
> +		struct exfat_dentry **file_ep)
> +{
> +	/* FIXME : is available in error case? */
> +	if (p_dir->dir == DIR_DELETED) {
> +		exfat_msg(sb, KERN_ERR, "access to deleted dentry\n");
> +		return NULL;
> +	}

Will this place need any further improvements?


…
> +	bh = sb_bread(sb, sec);
> +	if (!bh)
> +		goto err_out;

Can it be nicer to return directly?


…
> +	ep = (struct exfat_dentry *)(bh->b_data + off);
> +	entry_type = exfat_get_entry_type(ep);
> +
> +	if (entry_type != TYPE_FILE && entry_type != TYPE_DIR)
> +		goto err_out;

+		goto release_bh;

…
> +		if (!exfat_validate_entry(exfat_get_entry_type(ep), &mode))
> +			goto err_out;
> +	brelse(bh);
> +
> +	return es;
> +err_out:
> +	kfree(es);
> +	brelse(bh);

I suggest to improve the exception handling also for this function implementation.

+	brelse(bh);
+	return es;
+
+free_es:
+	kfree(es);
+release_bh:
+	brelse(bh);


* Would you like to adjust any more jump targets at similar places?

* Can another initialisation be omitted then for a pointer variable?

Regards,
Markus

  reply	other threads:[~2019-11-21 13:09 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20191121052913epcas1p1b28d2727dca5df42a6f2b8eb6b6dbcbb@epcas1p1.samsung.com>
2019-11-21  5:26 ` [PATCH v4 00/13] add the latest exfat driver Namjae Jeon
     [not found]   ` <CGME20191121052915epcas1p36613f6c468471c95d02da5db92b5c6cd@epcas1p3.samsung.com>
2019-11-21  5:26     ` [PATCH v4 01/13] exfat: add in-memory and on-disk structures and headers Namjae Jeon
     [not found]   ` <CGME20191121052915epcas1p30f42f12990926942d24aa514ebc437ac@epcas1p3.samsung.com>
2019-11-21  5:26     ` [PATCH v4 02/13] exfat: add super block operations Namjae Jeon
2019-11-21  9:40       ` Markus Elfring
     [not found]   ` <CGME20191121052916epcas1p3f00c8e510eb53f53f4e082848bd325d0@epcas1p3.samsung.com>
2019-11-21  5:26     ` [PATCH v4 03/13] exfat: add inode operations Namjae Jeon
2019-11-21 10:40       ` Markus Elfring
2019-11-21 11:51         ` Greg Kroah-Hartman
2019-11-22  6:33         ` Christoph Hellwig
     [not found]   ` <CGME20191121052917epcas1p259b8cb61ab86975cabc0cf4815a8dc38@epcas1p2.samsung.com>
2019-11-21  5:26     ` [PATCH v4 04/13] exfat: add directory operations Namjae Jeon
2019-11-21 13:07       ` Markus Elfring [this message]
2019-11-22  3:00         ` Namjae Jeon
2019-11-22  8:11           ` Markus Elfring
2019-11-22  8:21             ` Namjae Jeon
2019-11-22  8:34               ` Dan Carpenter
2019-11-22  8:52             ` Christoph Hellwig
2019-11-22 10:30               ` [v4 " Markus Elfring
     [not found]   ` <CGME20191121052917epcas1p1f81875dcc2d1a64dc3420bedc68fb3ba@epcas1p1.samsung.com>
2019-11-21  5:26     ` [PATCH v4 05/13] exfat: add file operations Namjae Jeon
2019-11-21 13:45       ` Markus Elfring
     [not found]   ` <CGME20191121052918epcas1p284be629c57ced93afe88b94a35437cb2@epcas1p2.samsung.com>
2019-11-21  5:26     ` [PATCH v4 06/13] exfat: add exfat entry operations Namjae Jeon
2019-11-21 14:10       ` Markus Elfring
     [not found]   ` <CGME20191121052918epcas1p31b6aa77d4642ea390e8390bc782d590e@epcas1p3.samsung.com>
2019-11-21  5:26     ` [PATCH v4 07/13] exfat: add bitmap operations Namjae Jeon
     [not found]   ` <CGME20191121052919epcas1p1de897233478ce6428df75176611cc338@epcas1p1.samsung.com>
2019-11-21  5:26     ` [PATCH v4 08/13] exfat: add exfat cache Namjae Jeon
     [not found]   ` <CGME20191121052920epcas1p2a38edd92dece3ad2cec74439175fcc52@epcas1p2.samsung.com>
2019-11-21  5:26     ` [PATCH v4 09/13] exfat: add misc operations Namjae Jeon
     [not found]   ` <CGME20191121052920epcas1p3e5b6c0251e869e265d19798dbeebab4e@epcas1p3.samsung.com>
2019-11-21  5:26     ` [PATCH v4 10/13] exfat: add nls operations Namjae Jeon
2019-11-21  8:09       ` Markus Elfring
2019-11-21  8:19         ` Namjae Jeon
     [not found]   ` <CGME20191121052921epcas1p35c720135e315a34d16800057e8e67829@epcas1p3.samsung.com>
2019-11-21  5:26     ` [PATCH v4 11/13] exfat: add Kconfig and Makefile Namjae Jeon
     [not found]   ` <CGME20191121052921epcas1p1f3a18589dce59939af6b462f35412642@epcas1p1.samsung.com>
2019-11-21  5:26     ` [PATCH v4 12/13] exfat: add exfat in fs/Kconfig and fs/Makefile Namjae Jeon
2019-11-23 15:52       ` kbuild test robot
2019-11-23 15:52       ` [PATCH] exfat: fix boolreturn.cocci warnings kbuild test robot
2019-11-24  2:09         ` Signed-off-by: (was " Valdis Klētnieks
2019-11-24  6:59           ` Greg KH
     [not found]   ` <CGME20191121052922epcas1p4c3957530e6d0ea89e1fc09322e999fb9@epcas1p4.samsung.com>
2019-11-21  5:26     ` [PATCH v4 13/13] MAINTAINERS: add exfat filesystem Namjae Jeon
2019-11-21  8:55   ` [PATCH v4 00/13] add the latest exfat driver Markus Elfring

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=498a958f-9066-09c6-7240-114234965c1a@web.de \
    --to=markus.elfring@web.de \
    --cc=dwagner@suse.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@lst.de \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linkinjeon@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namjae.jeon@samsung.com \
    --cc=nborisov@suse.com \
    --cc=sj1557.seo@samsung.com \
    --cc=valdis.kletnieks@vt.edu \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).