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
| ||
|
Message-ID: <ac6a4fd9-f2af-bad5-ce5d-ea728e9b64b2@huawei.com> Date: Tue, 18 Aug 2020 19:33:31 +0800 From: Shijie Luo <luoshijie1@...wei.com> To: Jan Kara <jack@...e.cz> CC: <linux-ext4@...r.kernel.org>, <tytso@....edu>, <linfeilong@...wei.com> Subject: Re: [PATCH] jbd2: remove useless variable chksum_seen in do_one_pass 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@...wei.com> > 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.
Powered by blists - more mailing lists