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

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