* [PATCH] xfs: Remove duplicated check
@ 2018-02-20 13:53 Nikolay Borisov
2018-02-20 16:39 ` Darrick J. Wong
2018-02-20 21:11 ` Dave Chinner
0 siblings, 2 replies; 4+ messages in thread
From: Nikolay Borisov @ 2018-02-20 13:53 UTC (permalink / raw)
To: linux-xfs; +Cc: Nikolay Borisov
The check performed before the memcpy responsible for copying the rest
of the inode is already performed before we call xfs_log_dinode_to_disk.
So let's remove the 2nd instance of the check. No functional changes.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reported-by: Jeff Mahoney <jeffm@suse.com>
---
fs/xfs/xfs_log_recover.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 00240c9ee72e..88dccfb1de96 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -3174,11 +3174,8 @@ xlog_recover_inode_pass2(
xfs_log_dinode_to_disk(ldip, dip);
/* the rest is in on-disk format */
- if (item->ri_buf[1].i_len > isize) {
- memcpy((char *)dip + isize,
- item->ri_buf[1].i_addr + isize,
+ memcpy((char *)dip + isize, item->ri_buf[1].i_addr + isize,
item->ri_buf[1].i_len - isize);
- }
fields = in_f->ilf_fields;
if (fields & XFS_ILOG_DEV)
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: Remove duplicated check
2018-02-20 13:53 [PATCH] xfs: Remove duplicated check Nikolay Borisov
@ 2018-02-20 16:39 ` Darrick J. Wong
2018-02-20 21:11 ` Dave Chinner
1 sibling, 0 replies; 4+ messages in thread
From: Darrick J. Wong @ 2018-02-20 16:39 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: linux-xfs
On Tue, Feb 20, 2018 at 03:53:02PM +0200, Nikolay Borisov wrote:
> The check performed before the memcpy responsible for copying the rest
> of the inode is already performed before we call xfs_log_dinode_to_disk.
> So let's remove the 2nd instance of the check. No functional changes.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> Reported-by: Jeff Mahoney <jeffm@suse.com>
Looks ok, will test...
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/xfs/xfs_log_recover.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 00240c9ee72e..88dccfb1de96 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -3174,11 +3174,8 @@ xlog_recover_inode_pass2(
> xfs_log_dinode_to_disk(ldip, dip);
>
> /* the rest is in on-disk format */
> - if (item->ri_buf[1].i_len > isize) {
> - memcpy((char *)dip + isize,
> - item->ri_buf[1].i_addr + isize,
> + memcpy((char *)dip + isize, item->ri_buf[1].i_addr + isize,
> item->ri_buf[1].i_len - isize);
> - }
>
> fields = in_f->ilf_fields;
> if (fields & XFS_ILOG_DEV)
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: Remove duplicated check
2018-02-20 13:53 [PATCH] xfs: Remove duplicated check Nikolay Borisov
2018-02-20 16:39 ` Darrick J. Wong
@ 2018-02-20 21:11 ` Dave Chinner
2018-02-21 6:30 ` Nikolay Borisov
1 sibling, 1 reply; 4+ messages in thread
From: Dave Chinner @ 2018-02-20 21:11 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: linux-xfs
On Tue, Feb 20, 2018 at 03:53:02PM +0200, Nikolay Borisov wrote:
> The check performed before the memcpy responsible for copying the rest
> of the inode is already performed before we call xfs_log_dinode_to_disk.
> So let's remove the 2nd instance of the check. No functional changes.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> Reported-by: Jeff Mahoney <jeffm@suse.com>
> ---
> fs/xfs/xfs_log_recover.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 00240c9ee72e..88dccfb1de96 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -3174,11 +3174,8 @@ xlog_recover_inode_pass2(
> xfs_log_dinode_to_disk(ldip, dip);
>
> /* the rest is in on-disk format */
> - if (item->ri_buf[1].i_len > isize) {
> - memcpy((char *)dip + isize,
> - item->ri_buf[1].i_addr + isize,
> + memcpy((char *)dip + isize, item->ri_buf[1].i_addr + isize,
> item->ri_buf[1].i_len - isize);
> - }
>
This looks wrong.
The previous check is:
if (unlikely(item->ri_buf[1].i_len > isize)) {
CORRUPTION_ERROR
....
error = -EFSCORRUPTED;
goto out_release;
}
So after this item->ri_buf[1].i_len is always <= isize. IOWs, the
memcpy() is currently dead code that is never executed, not code we
want to execute in every inode recovery.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: Remove duplicated check
2018-02-20 21:11 ` Dave Chinner
@ 2018-02-21 6:30 ` Nikolay Borisov
0 siblings, 0 replies; 4+ messages in thread
From: Nikolay Borisov @ 2018-02-21 6:30 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On 20.02.2018 23:11, Dave Chinner wrote:
> On Tue, Feb 20, 2018 at 03:53:02PM +0200, Nikolay Borisov wrote:
>> The check performed before the memcpy responsible for copying the rest
>> of the inode is already performed before we call xfs_log_dinode_to_disk.
>> So let's remove the 2nd instance of the check. No functional changes.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> Reported-by: Jeff Mahoney <jeffm@suse.com>
>> ---
>> fs/xfs/xfs_log_recover.c | 5 +----
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
>> index 00240c9ee72e..88dccfb1de96 100644
>> --- a/fs/xfs/xfs_log_recover.c
>> +++ b/fs/xfs/xfs_log_recover.c
>> @@ -3174,11 +3174,8 @@ xlog_recover_inode_pass2(
>> xfs_log_dinode_to_disk(ldip, dip);
>>
>> /* the rest is in on-disk format */
>> - if (item->ri_buf[1].i_len > isize) {
>> - memcpy((char *)dip + isize,
>> - item->ri_buf[1].i_addr + isize,
>> + memcpy((char *)dip + isize, item->ri_buf[1].i_addr + isize,
>> item->ri_buf[1].i_len - isize);
>> - }
>>
>
> This looks wrong.
>
> The previous check is:
>
> if (unlikely(item->ri_buf[1].i_len > isize)) {
> CORRUPTION_ERROR
> ....
> error = -EFSCORRUPTED;
> goto out_release;
> }
>
> So after this item->ri_buf[1].i_len is always <= isize. IOWs, the
> memcpy() is currently dead code that is never executed, not code we
> want to execute in every inode recovery.
Doh, you are right, will resend.
>
> Cheers,
>
> Dave.
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-02-21 6:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-20 13:53 [PATCH] xfs: Remove duplicated check Nikolay Borisov
2018-02-20 16:39 ` Darrick J. Wong
2018-02-20 21:11 ` Dave Chinner
2018-02-21 6:30 ` Nikolay Borisov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.