Messages in this thread | | | Subject | Re: [PATCH] jffs2: Don't add summary entry when MTD write fails | From | Thomas.Betker@rohde-sc ... | Date | Thu, 25 Feb 2016 18:48:56 +0100 |
| |
Hello David:
> > + int ret; > > + > > + ret = mtd_writev(c->mtd, vecs, count, to, retlen); > > + > > if (!jffs2_is_writebuffered(c)) { > > if (jffs2_sum_active()) { > > int res; > > + > > + if (ret || > > + *retlen != iov_length((struct iovec *) > vecs, count)) > > + return ret; > > + > > res = jffs2_sum_add_kvec(c, vecs, count, > (uint32_t) to); > > if (res) { > > return res; > > OK... but perhaps we can dispense with the separate 'ret' and 'res' > variables and the rats nest of conditions, and do something like: > > int ret; > > ret = mtd_writev(…); > > if (!ret && *retlen == iov_length(…) && > !jffs2_is_writebuffered(c) && jffs2_sum_active()) > ret = jffs2_sum_add_kvec(…); > > return ret;
While the logic is the same, will the compiler generate the same code? When CONFIG_JFFS2_SUMMARY is not set, "if (jffs2_sum_active())" means "if (0)", and I would assume that the compiler removes the whole clause, "if" and all. However, I am not sure what happens with "if (!ret && whatever && 0)".
That's why I was taking pains to keep the original control flow intact, even if it's a rat's nest (it is). If I remember correctly, jffs2_flash_direct_writev() is called quite often, and I didn't want performance to suffer. I may be completely wrong here, of course, but then why wasn't the original source code "if (!jffs2_is_writebuffered(c) && jffs2_sum_active())"?
Best regards, Thomas
| |