Re: [PATCH v10 03/15] FWU: Add FWU metadata access driver for GPT partitioned block devices

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

Hey Etiennem

[...]
> > > > +                                 uint *secondary_mpart)
> > > > +{
> > > > +     int i, ret;
> > > > +     u32 mdata_parts;
> > > > +     efi_guid_t part_type_guid;
> > > > +     struct disk_partition info;
> > > > +     const efi_guid_t fwu_mdata_guid = FWU_MDATA_GUID;
> > > > +
> > > > +     mdata_parts = 0;
> > > > +     for (i = 1; i < MAX_SEARCH_PARTITIONS; i++) {
> > > > +             if (part_get_info(desc, i, &info))
> > > > +                     continue;
> > > > +             uuid_str_to_bin(info.type_guid, part_type_guid.b,
> > > > +                             UUID_STR_FORMAT_GUID);
> > > > +
> > > > +             if (!guidcmp(&fwu_mdata_guid, &part_type_guid)) {
> > > > +                     ++mdata_parts;
> > > > +                     if (!*primary_mpart)
> > > > +                             *primary_mpart = i;
> > > > +                     else
> > > > +                             *secondary_mpart = i;
> > > > +             }
> > > > +     }
> > > > +
> > > > +     if (mdata_parts != 2) {
> > > > +             log_debug("Expect two copies of the FWU metadata instead of %d\n",
> > > > +                       mdata_parts);
> > > > +             ret = -EINVAL;
> > > > +     } else {
> > > > +             ret = 0;
> > > > +     }
> > >
> > > Can we change that a bit?  There are are some assumptions in this code, e.g
> > > the user must pass the values zero'ed out.  Can we instead get an array of
> 
> I don't agree. If the function returns an error code, caller shall not
> use the output content.
> That is what callers of gpt_get_mdata_partitions() do in this patch,
> so returning -EINVAL if fine.
> (note that I don't mind this gets an array ref rather than 2 pointers).

Ah true. In any case I still think we should change to an array,  imho it
would be much easier to read

> 
> best regards,
> etienne
> 
> > > 2 instead?  In that case you don't care if any of the values are zero and
> > > you can just fill in the array.  Something like:
> >
> > Okay, will make use of the array as you suggest.
> >
> > >

[...]

Regards
/Ilias