On Mon, Nov 7, 2022 at 2:13 PM Etienne Carriere etienne.carriere@linaro.org wrote:
Hello Jassi,
On Mon, 7 Nov 2022 at 19:29, Jassi Brar jassisinghbrar@gmail.com wrote:
On Mon, Nov 7, 2022 at 11:24 AM Etienne Carriere etienne.carriere@linaro.org wrote:
On Fri, 4 Nov 2022 at 03:43, jassisinghbrar@gmail.com wrote:
+/**
- fwu_get_verified_mdata() - Read, verify and return the FWU metadata
- Read both the metadata copies from the storage media, verify their checksum,
- and ascertain that both copies match. If one of the copies has gone bad,
- restore it from the good copy.
- Return: 0 if OK, -ve on error
+*/ +int fwu_get_verified_mdata(struct fwu_mdata *mdata);
Nitpicking: would you be ok to rename this function to fwu_get_mdata(). When getting fwu mdata, we obviously expect to get reliable data.
I originally called it fwu_get_mdata() in my local 1-patch change :) But there already is one such function and I had to rename, only to delete that.
Ok, maybe a later commit to rename the function once the old one has been removed in patch 4/4.
ok.
....
+static struct fwu_mdata g_mdata = { 0 };
Can remove "= { 0 };"
I knew, but it was supposed to imply that we want the first crc check to fail (because we set that to 0 here).
As zero init is implicit, I think maintainers will ask to remove it here.
Though there are many such examples already and the compiler would anyway ignore it.
Maybe add an inline comment above the global variable definition. /* Crc32 of a zeroes produces a non 0 value */
ok.
thanks.