btrfs-progs: check: Initialize all filed of btrfs_inode_item in insert_inode_item()
diff mbox

Message ID a9e2495c-6085-4efb-5257-f58a5019e2b7@jp.fujitsu.com
State New
Headers show

Commit Message

Misono Tomohiro June 7, 2018, 2:49 a.m. UTC
Initialize all filed of btrfs_inode_item to zero in order to prevent
having some garbage, especially for flags field.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 check/mode-common.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

David Sterba June 7, 2018, 12:22 p.m. UTC | #1
On Thu, Jun 07, 2018 at 11:49:58AM +0900, Misono Tomohiro wrote:
> Initialize all filed of btrfs_inode_item to zero in order to prevent
> having some garbage, especially for flags field.

Have you observed in practice or is it a matter of precaution?
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Misono Tomohiro June 8, 2018, 12:43 a.m. UTC | #2
On 2018/06/07 21:22, David Sterba wrote:
> On Thu, Jun 07, 2018 at 11:49:58AM +0900, Misono Tomohiro wrote:
>> Initialize all filed of btrfs_inode_item to zero in order to prevent
>> having some garbage, especially for flags field.
> 
> Have you observed in practice or is it a matter of precaution?

I saw failure of fsck-test/010 in yesterday's devel branch and
made this patch. It turned out that root cause was wrong flag comparison
in btrfs check.
(https://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg77758.html)

With Su's fix, failure of fsck-test/010 is also gone without this patch,
but it is better to initialize the variables anyway.

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Su Yue June 8, 2018, 9:04 a.m. UTC | #3
On 06/08/2018 08:43 AM, Misono Tomohiro wrote:
> On 2018/06/07 21:22, David Sterba wrote:
>> On Thu, Jun 07, 2018 at 11:49:58AM +0900, Misono Tomohiro wrote:
>>> Initialize all filed of btrfs_inode_item to zero in order to prevent
>>> having some garbage, especially for flags field.
>>
>> Have you observed in practice or is it a matter of precaution?
> 
> I saw failure of fsck-test/010 in yesterday's devel branch and
> made this patch. It turned out that root cause was wrong flag comparison
> in btrfs check.
> (https://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg77758.html)
> 
> With Su's fix, failure of fsck-test/010 is also gone without this patch,
> but it is better to initialize the variables anyway.
> 
Agreed. I saw odd flags reported by btrfs check too.
The callers of insert_inode_item() don't set inode flags manully.

> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/check/mode-common.c b/check/mode-common.c
index db9e4299..15e2bbd1 100644
--- a/check/mode-common.c
+++ b/check/mode-common.c
@@ -379,18 +379,14 @@  int insert_inode_item(struct btrfs_trans_handle *trans,
 	time_t now = time(NULL);
 	int ret;
 
+	memset(&ii, 0, sizeof(ii));
 	btrfs_set_stack_inode_size(&ii, size);
 	btrfs_set_stack_inode_nbytes(&ii, nbytes);
 	btrfs_set_stack_inode_nlink(&ii, nlink);
 	btrfs_set_stack_inode_mode(&ii, mode);
 	btrfs_set_stack_inode_generation(&ii, trans->transid);
-	btrfs_set_stack_timespec_nsec(&ii.atime, 0);
 	btrfs_set_stack_timespec_sec(&ii.ctime, now);
-	btrfs_set_stack_timespec_nsec(&ii.ctime, 0);
 	btrfs_set_stack_timespec_sec(&ii.mtime, now);
-	btrfs_set_stack_timespec_nsec(&ii.mtime, 0);
-	btrfs_set_stack_timespec_sec(&ii.otime, 0);
-	btrfs_set_stack_timespec_nsec(&ii.otime, 0);
 
 	ret = btrfs_insert_inode(trans, root, ino, &ii);
 	ASSERT(!ret);