On Mon, Nov 7, 2022 at 2:13 PM Etienne Carriere
<etienne.carri...@linaro.org> wrote:
>
> Hello Jassi,
>
> On Mon, 7 Nov 2022 at 19:29, Jassi Brar <jassisinghb...@gmail.com> wrote:
> >
> > On Mon, Nov 7, 2022 at 11:24 AM Etienne Carriere
> > <etienne.carri...@linaro.org> wrote:
> > > On Fri, 4 Nov 2022 at 03:43, <jassisinghb...@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.
