Re: [PATCH]fs/jffs2/wbuf.c: add compatibility support for OOB data block

Artem B. Bityuckiy
2005-08-16T11:35:25Z

And one more question:

> + if (je32_to_cpu(summary->hdr_crc) != crc) {
> + D1(printk(KERN_DEBUG "jffs2_scan_eraseblock(): Summary node
> header is corrupt (bad CRC or no summary at all)\n"));
> + bad_sum = 1;
> + }
Ok, we have bad CRC, why do we continue checking the other CRCs?

> +
> + if ((!bad_sum) && (je32_to_cpu(summary->totlen) != sumsize)) {
> + D1(printk(KERN_DEBUG "jffs2_scan_eraseblock(): Summary node
is
> corrupt (wrong erasesize?)\n"));
> + bad_sum = 1;
> + }
The same, why do not use goto here?

> +
> + crc = crc32(0, summary, sizeof(struct jffs2_summary_node)-8);
> +
> + if ((!bad_sum) && (je32_to_cpu(summary->node_crc) != crc)) {
> + D1(printk(KERN_DEBUG "jffs2_scan_eraseblock(): Summary node
is
> corrupt (bad CRC)\n"));
> + bad_sum = 1;
> + }
Here.

> +
> + crc = crc32(0, summary->sum, sumsize - sizeof(struct
> jffs2_summary_node));
> +
> + if ((!bad_sum) && (je32_to_cpu(summary->sum_crc) != crc)) {
> + D1(printk(KERN_DEBUG "jffs2_scan_eraseblock(): Summary node
> data is corrupt (bad CRC)\n"));
> + bad_sum = 1;
> + }
Here.

> +
> + if (!bad_sum) {
> +
> + struct jffs2_sum_unknown_flash *sp;
> + sp = (struct jffs2_sum_unknown_flash *) summary->sum;
Now we have:

if (!bad_sum) {
Huge block where we waste one indent! What for ?
}

return 0;

Why not to do like this:

if (1st crc check fails ())
goto fail;

if (2nd crc check fails ())
goto fail;

if (3rd crc check fails ())
goto fail;

process summary with good CRC ();

fail:

return 0;

And we save one indent which is rather expensive in Linux :-)

--
Best Regards,
Artem B. Bityuckiy,
St.-Petersburg, Russia.