On 2020/8/18 18:48, Jan Kara wrote:
On Mon 10-08-20 22:21:28, Shijie Luo wrote:
This variable only indicates that we do checksum success, while
chksum_err can also do. Moreover, condition "!chksum_seen" in else
if bracket is pointless.
Signed-off-by: Shijie Luo <luoshijie1@xxxxxxxxxx>
Thanks for the patch! Some comments below.
@@ -709,11 +707,10 @@ static int do_one_pass(journal_t *journal,
cbh->h_chksum_type == JBD2_CRC32_CHKSUM &&
cbh->h_chksum_size ==
JBD2_CRC32_CHKSUM_SIZE)
- chksum_seen = 1;
+ chksum_err = 0;
else if (!(cbh->h_chksum_type == 0 &&
cbh->h_chksum_size == 0 &&
- found_chksum == 0 &&
- !chksum_seen))
+ found_chksum == 0))
/*
* If fs is mounted using an old kernel and then
* kernel with journal_chksum is used then we
I agree the use of chksum_err & chksum_seen looks rather arbitrary. In fact
the code seems to be equivalent to:
/* Neither checksum match nor unused? */
if (!(crc32_sum == found_chksum &&
cbh->h_chksum_type == JBD2_CRC32_CHKSUM &&
cbh->h_chksum_size ==
JBD2_CRC32_CHKSUM_SIZE) &&
!(cbh->h_chksum_type == 0 &&
cbh->h_chksum_size == 0 &&
found_chksum == 0)) {
info->end_transaction = next_commit_ID;
if (jbd2_has_feature_async_commit(journal)) {
...
}
}
crc32_sum = ~0;
which would be even simpler...
Honza
Thanks for your review，it 's definitely true to use these code as a
substitute, but I think these are
a little bit hard to read. By the way, I found a indentation error on
"chksum_err = 1".
Do you think which one is better? I will take your opinion into account
and send a v2 patch.
