Re: [PATCHv4 2/5] fwu: move meta-data management in core

Hi Ilias,

On Thu, Feb 23, 2023 at 2:36 AM Ilias Apalodimas ilias.apalodimas@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.