On Sun, Jun 23, 2024 at 07:34:49AM +0200, Christoph Hellwig wrote:
> While ->release returns int, the only caller ignores the return value.
> As we're only doing cleanup work there isn't much of a point in
> return a value to start with, so just document the situation instead.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/xfs_file.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index d39d0ea522d1c2..7b91cbab80da55 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1186,6 +1186,10 @@ xfs_dir_open(
> return error;
> }
>
> +/*
> + * Don't bother propagating errors. We're just doing cleanup, and the caller
> + * ignores the return value anyway.
Shouldn't we drop the int return from the function declaration, then?
(Is that also a cleanup that's you're working on?)
--D
> + */
> STATIC int
> xfs_file_release(
> struct inode *inode,
> @@ -1193,7 +1197,6 @@ xfs_file_release(
> {
> struct xfs_inode *ip = XFS_I(inode);
> struct xfs_mount *mp = ip->i_mount;
> - int error;
>
> /* If this is a read-only mount, don't generate I/O */
> if (xfs_is_readonly(mp))
> @@ -1211,11 +1214,8 @@ xfs_file_release(
> if (!xfs_is_shutdown(mp) &&
> xfs_iflags_test_and_clear(ip, XFS_ITRUNCATED)) {
> xfs_iflags_clear(ip, XFS_IDIRTY_RELEASE);
> - if (ip->i_delayed_blks > 0) {
> - error = filemap_flush(inode->i_mapping);
> - if (error)
> - return error;
> - }
> + if (ip->i_delayed_blks > 0)
> + filemap_flush(inode->i_mapping);
> }
>
> /*
> @@ -1249,14 +1249,14 @@ xfs_file_release(
> * dirty close we will still remove the speculative
> * allocation, but after that we will leave it in place.
> */
> - error = xfs_free_eofblocks(ip);
> - if (!error && ip->i_delayed_blks)
> + xfs_free_eofblocks(ip);
> + if (ip->i_delayed_blks)
> xfs_iflags_set(ip, XFS_IDIRTY_RELEASE);
> }
> xfs_iunlock(ip, XFS_IOLOCK_EXCL);
> }
>
> - return error;
> + return 0;
> }
>
> STATIC int
> --
> 2.43.0
>
>
