Hi Heinrich,
- efi_status_t ret;
- void *buf = NULL;
- *size = 0;
- ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
- if (ret == EFI_BUFFER_TOO_SMALL) {
buf = malloc(*size);Please, always check the output of malloc(), e.g.
if (!buf) ret = EFI_OUT_OF_RESOURCES; else
I just moved the function from lib/efi_loader/efi_bootmgr.c (check for get_var()) and completely missed that. I'll fix it on the next revision.
ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);- }
- if (ret != EFI_SUCCESS) {
free(buf);*size = 0;return NULL;- }
- return buf;
+}
+/**
- efi_dp_instance_by_idx() - Get a file path with a specific index
- @name: device path array
- @size: size of the discovered device path
- @idx: index of the device path
- Return: device path or NULL. Caller must free the returned value
- */
+struct +efi_device_path *efi_dp_instance_by_idx(struct efi_device_path *dp,
efi_uintn_t *size, int idx)+{
idx should be of an unsigned type as we cannot have negative indices.
- struct efi_device_path *instance = NULL;
- efi_uintn_t instance_size = 0;
- if (!efi_dp_is_multi_instance(dp))
Given a device path with one instance, why don't you allow me to read index 0? I assume this check can be removed.
Yea, but why would you ever use that? you can just retrieve the deserialized device path directly if there are no other instances.
return NULL;- while (idx >= 0 && dp) {
instance = efi_dp_get_next_instance(&dp, &instance_size);if (idx && instance) {efi_free_pool(instance);instance_size = 0;instance = NULL;}idx--;- }
- *size = instance_size;
- return instance;
This can be simplified with unsigned idx:
for (; dp; --idx) { instance = efi_dp_get_next_instance(&dp, size); if (!idx) { return instance; efi_free_pool(instance); } return NULL;
Ok I'll have a look
+}
+/**
- create_boot_var_indexed() - Return Boot#### name were #### is replaced by
the value of BootCurrent
- @var_name: variable name
- @var_name_size: size of var_name
- Return: Status code
- */
+static efi_status_t create_boot_var_indexed(u16 var_name[], size_t var_name_size) +{
- efi_uintn_t boot_order_size;
You are reading BootCurrent, not BootOrder.
- efi_status_t ret;
- u16 boot_order;
Same here.
Ok
- if (!file_path) {
printf("Instance not found\n");This message is not enough for a user to take action. I suggest
("Missing file path with index %d in %ls", idx, var_name)
We want to use logging. I assume this is not an error. Can we use log_debug() here?
That's a leftover from my opwn debugging that I neglected to remove prior to the RFC. I'll just add a log_debug()
return NULL;- }
- return file_path;
+}
Some other functions would fit into the same C module. Candidates are:
- efi_create_indexed_name()
- efi_deserialize_load_option()
- efi_serialize_load_option()
But that can be done in a separate patch.
Yea, that's the idea, we can also use the efi_get_var() in multiple places, but I'll send different patches for that, once we merge this.
I assume we agree on the architecture. If so I'll fix the selftests and post a proper patch
Regards /Ilias
Best regards
Heinrich