linux-ext4 - Re: [RFC PATCH v4 1/4] jbd2: introduce/export functions jbd2_journal_submit|finish_inode_data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <2A963475-D65D-4E58-9EDD-93D6784934B4@dilger.ca>
Date:   Mon, 28 Sep 2020 20:24:11 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     Mauricio Faria de Oliveira <mfo@...onical.com>
Cc:     Jan Kara <jack@...e.cz>, linux-ext4@...r.kernel.org,
        dann frazier <dann.frazier@...onical.com>
Subject: Re: [RFC PATCH v4 1/4] jbd2: introduce/export functions
 jbd2_journal_submit|finish_inode_data_buffers()

On Sep 28, 2020, at 1:41 PM, Mauricio Faria de Oliveira <mfo@...onical.com> wrote:
> 
> Export functions that implement the current behavior done
> for an inode in journal_submit|finish_inode_data_buffers().
> 
> No functional change.
> 
> Signed-off-by: Mauricio Faria de Oliveira <mfo@...onical.com>
> Suggested-by: Jan Kara <jack@...e.cz>
> Reviewed-by: Jan Kara <jack@...e.cz>

A couple of minor cleanups below, but either way you could add:

Reviewed-by: Andreas Dilger <adilger@...ger.ca>

> +int jbd2_journal_finish_inode_data_buffers(struct jbd2_inode *jinode)
> +{
> +	struct address_space *mapping = jinode->i_vfs_inode->i_mapping;
> +	loff_t dirty_start = jinode->i_dirty_start;
> +	loff_t dirty_end = jinode->i_dirty_end;
> +	int ret;
> +
> +	ret = filemap_fdatawait_range_keep_errors(mapping, dirty_start, dirty_end);
> +	return ret;
> +}

(style) still prefer to wrap at 80 columns if possible.
(style) there isn't any benefit to "dirty_start" and "dirty_end" as locals
(style) there also isn't any benefit to "ret = ...; return ret"

I thought it might be coded this way because the function is changed in a
later patch in the series, but I couldn't find anything like that, so the
shorter form is just as readable, IMHO:

int jbd2_journal_finish_inode_data_buffers(struct jbd2_inode *jinode)
{
	struct address_space *mapping = jinode->i_vfs_inode->i_mapping;

	return filemap_fdatawait_range_keep_errors(mapping,
						   jinode->dirty_start,
						   jinode->dirty_end);
}

Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)