Hi Ilias, On Thu, Feb 23, 2023 at 2:36 AM Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > +int fwu_get_verified_mdata(struct fwu_mdata *mdata) > > +{ > > + int err; > > + bool pri_ok, sec_ok; > > + struct fwu_mdata s, *p_mdata, *s_mdata; > > + > > + p_mdata = &g_mdata; > > + s_mdata = &s; > > + > > + /* if mdata already read and ready */ > > + err = mdata_crc_check(p_mdata); > > + if (!err) > > + goto ret_mdata; > > + /* else read, verify and, if needed, fix mdata */ > > + > > + pri_ok = false; > > + err = fwu_read_mdata(g_dev, p_mdata, true); > > + if (!err) { > > + err = mdata_crc_check(p_mdata); > > + if (!err) > > + pri_ok = true; > > + else > > + log_debug("primary mdata: crc32 failed\n"); > > + } > > + > > + sec_ok = false; > > + err = fwu_read_mdata(g_dev, s_mdata, false); > > + if (!err) { > > + err = mdata_crc_check(s_mdata); > > + if (!err) > > + sec_ok = true; > > + else > > + log_debug("secondary mdata: crc32 failed\n"); > > + } > > Isn't it better to define pri_ok, sec_ok and their equivalent mdata as > arrays ? IOW something along the lines of > > bool parts_ok[2] = { false }; > struct fwu_mdata parts_mdata[2]; > > parts_mdata[0] = &g_mdata; > parts_mdata[1] = ..... > for (i = 0; i < 2; i++) { > err = fwu_read_mdata(g_dev, parts_mdata[i], !(i % 2) ? true : false); > if (!err) > err = mdata_crc_check(parts_mdata[i]); > etc.... > } > > > + > > + if (pri_ok && sec_ok) { > > And then also adjust this part? > > > + /* > > + * Before returning, check that both the > > + * FWU metadata copies are the same. > > + */ > > + err = memcmp(p_mdata, s_mdata, sizeof(struct fwu_mdata)); > > + if (!err) > > + goto ret_mdata; > > + > > + /* > > + * If not, populate the secondary partition from the > > + * primary partition copy. > > + */ > > + log_info("Both FWU metadata copies are valid but do not > > match."); > > + log_info(" Restoring the secondary partition from the > > primary\n"); > > + sec_ok = false; > > + } > > + > > + if (!pri_ok) { > > + memcpy(p_mdata, s_mdata, sizeof(struct fwu_mdata)); > > + err = fwu_sync_mdata(p_mdata, PRIMARY_PART); > > + if (err) { > > + log_debug("mdata : primary write failed\n"); > > + return err; > > + } > > + } > > + > > + if (!sec_ok) { > > + memcpy(s_mdata, p_mdata, sizeof(struct fwu_mdata)); > > + err = fwu_sync_mdata(s_mdata, SECONDARY_PART); > > + if (err) { > > + log_debug("mdata : secondary write failed\n"); > > + return err; > > + } > > + } > > And this could also be folded into a for loop > I have done these modifications and submitted v5.
Thanks.