On 12.10.17 00:02, Rob Clark wrote:
On Wed, Oct 11, 2017 at 10:30 AM, Alexander Graf agraf@suse.de wrote:
On 10.10.17 14:22, Rob Clark wrote:
From: Leif Lindholm leif.lindholm@linaro.org
Enough implementation of the following protocols to run Shell.efi and SCT.efi:
EfiHiiConfigRoutingProtocolGuid EfiHiiDatabaseProtocol EfiHiiStringProtocol
We'll fill in the rest once SCT is running properly so we can validate the implementation against the conformance test suite.
Initial skeleton written by Leif, and then implementation by myself.
Cc: Leif Lindholm leif.lindholm@linaro.org Signed-off-by: Rob Clark robdclark@gmail.com
include/efi_api.h | 261 ++++++++++++++++++++++ include/efi_loader.h | 6 + lib/efi_loader/Makefile | 2 +- lib/efi_loader/efi_boottime.c | 9 + lib/efi_loader/efi_hii.c | 507 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 784 insertions(+), 1 deletion(-) create mode 100644 lib/efi_loader/efi_hii.c
diff --git a/include/efi_api.h b/include/efi_api.h index ffdba7fe1a..164147dc87 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -16,6 +16,7 @@ #define _EFI_API_H
#include <efi.h> +#include <charset.h>
#ifdef CONFIG_EFI_LOADER #include <asm/setjmp.h> @@ -536,6 +537,266 @@ struct efi_device_path_utilities_protocol { uint16_t node_length); };
+#define EFI_HII_CONFIG_ROUTING_PROTOCOL_GUID \
EFI_GUID(0x587e72d7, 0xcc50, 0x4f79, \0x82, 0x09, 0xca, 0x29, 0x1f, 0xc1, 0xa1, 0x0f)+typedef uint16_t efi_string_id_t;
+struct efi_hii_config_routing_protocol {
efi_status_t(EFIAPI *extract_config)(const struct efi_hii_config_routing_protocol *this,const efi_string_t request,efi_string_t *progress,efi_string_t *results);efi_status_t(EFIAPI *export_config)(const struct efi_hii_config_routing_protocol *this,efi_string_t *results);efi_status_t(EFIAPI *route_config)(const struct efi_hii_config_routing_protocol *this,const efi_string_t configuration,efi_string_t *progress);efi_status_t(EFIAPI *block_to_config)(const struct efi_hii_config_routing_protocol *this,const efi_string_t config_request,const uint8_t *block,const efi_uintn_t block_size,efi_string_t *config,efi_string_t *progress);efi_status_t(EFIAPI *config_to_block)(const struct efi_hii_config_routing_protocol *this,const efi_string_t config_resp,const uint8_t *block,const efi_uintn_t *block_size,efi_string_t *progress);efi_status_t(EFIAPI *get_alt_config)(const struct efi_hii_config_routing_protocol *this,const efi_string_t config_resp,const efi_guid_t *guid,const efi_string_t name,const struct efi_device_path *device_path,const efi_string_t alt_cfg_id,efi_string_t *alt_cfg_resp);+};
+#define EFI_HII_DATABASE_PROTOCOL_GUID \
EFI_GUID(0xef9fc172, 0xa1b2, 0x4693, \0xb3, 0x27, 0x6d, 0x32, 0xfc, 0x41, 0x60, 0x42)+typedef enum {
EFI_KEY_LCTRL, EFI_KEY_A0, EFI_KEY_LALT, EFI_KEY_SPACE_BAR,EFI_KEY_A2, EFI_KEY_A3, EFI_KEY_A4, EFI_KEY_RCTRL, EFI_KEY_LEFT_ARROW,EFI_KEY_DOWN_ARROW, EFI_KEY_RIGHT_ARROW, EFI_KEY_ZERO,EFI_KEY_PERIOD, EFI_KEY_ENTER, EFI_KEY_LSHIFT, EFI_KEY_B0,EFI_KEY_B1, EFI_KEY_B2, EFI_KEY_B3, EFI_KEY_B4, EFI_KEY_B5, EFI_KEY_B6,EFI_KEY_B7, EFI_KEY_B8, EFI_KEY_B9, EFI_KEY_B10, EFI_KEY_RSHIFT,EFI_KEY_UP_ARROW, EFI_KEY_ONE, EFI_KEY_TWO, EFI_KEY_THREE,EFI_KEY_CAPS_LOCK, EFI_KEY_C1, EFI_KEY_C2, EFI_KEY_C3, EFI_KEY_C4,EFI_KEY_C5, EFI_KEY_C6, EFI_KEY_C7, EFI_KEY_C8, EFI_KEY_C9,EFI_KEY_C10, EFI_KEY_C11, EFI_KEY_C12, EFI_KEY_FOUR, EFI_KEY_FIVE,EFI_KEY_SIX, EFI_KEY_PLUS, EFI_KEY_TAB, EFI_KEY_D1, EFI_KEY_D2,EFI_KEY_D3, EFI_KEY_D4, EFI_KEY_D5, EFI_KEY_D6, EFI_KEY_D7, EFI_KEY_D8,EFI_KEY_D9, EFI_KEY_D10, EFI_KEY_D11, EFI_KEY_D12, EFI_KEY_D13,EFI_KEY_DEL, EFI_KEY_END, EFI_KEY_PG_DN, EFI_KEY_SEVEN, EFI_KEY_EIGHT,EFI_KEY_NINE, EFI_KEY_E0, EFI_KEY_E1, EFI_KEY_E2, EFI_KEY_E3,EFI_KEY_E4, EFI_KEY_E5, EFI_KEY_E6, EFI_KEY_E7, EFI_KEY_E8, EFI_KEY_E9,EFI_KEY_E10, EFI_KEY_E11, EFI_KEY_E12, EFI_KEY_BACK_SPACE,EFI_KEY_INS, EFI_KEY_HOME, EFI_KEY_PG_UP, EFI_KEY_NLCK, EFI_KEY_SLASH,EFI_KEY_ASTERISK, EFI_KEY_MINUS, EFI_KEY_ESC, EFI_KEY_F1, EFI_KEY_F2,EFI_KEY_F3, EFI_KEY_F4, EFI_KEY_F5, EFI_KEY_F6, EFI_KEY_F7, EFI_KEY_F8,EFI_KEY_F9, EFI_KEY_F10, EFI_KEY_F11, EFI_KEY_F12, EFI_KEY_PRINT,EFI_KEY_SLCK, EFI_KEY_PAUSE,+} efi_key;
+struct efi_key_descriptor {
efi_key key;uint16_t unicode;uint16_t shifted_unicode;uint16_t alt_gr_unicode;uint16_t shifted_alt_gr_unicode;uint16_t modifier;uint16_t affected_attribute;+};
+struct efi_hii_keyboard_layout {
uint16_t layout_length;efi_guid_t guid;uint32_t layout_descriptor_string_offset;uint8_t descriptor_count;struct efi_key_descriptor descriptors[];+};
+struct efi_hii_package_list_header {
efi_guid_t package_list_guid;uint32_t package_length;+} __packed;
+struct efi_hii_package_header {
uint32_t length : 24;uint32_t type : 8;+} __packed;
Bitfields are terribly evil - they're probably one of the worst defined things in C. A different compiler may even give you different ordering here. I've certainly seen bitfields explode in weird ways on cross-endian conversions.
edk2 defines it in the same way. And this is UEFI we are talking about here, big endian is strictly out of scope.
I don't see why big endian is strictly out of scope. I don't want to ignore it light heartedly. All we'd need to do is byte swap every input/output there is today.
So please change it to not use bitmasks. I don't think we should copy bad design decisions from edk2.
Do you think you could just make that a uint32_t altogether and work with MASK/SHIFT defines instead?
+#define EFI_HII_PACKAGE_TYPE_ALL 0x00 +#define EFI_HII_PACKAGE_TYPE_GUID 0x01 +#define EFI_HII_PACKAGE_FORMS 0x02 +#define EFI_HII_PACKAGE_STRINGS 0x04 +#define EFI_HII_PACKAGE_FONTS 0x05 +#define EFI_HII_PACKAGE_IMAGES 0x06 +#define EFI_HII_PACKAGE_SIMPLE_FONTS 0x07 +#define EFI_HII_PACKAGE_DEVICE_PATH 0x08 +#define EFI_HII_PACKAGE_KEYBOARD_LAYOUT 0x09 +#define EFI_HII_PACKAGE_ANIMATIONS 0x0A +#define EFI_HII_PACKAGE_END 0xDF +#define EFI_HII_PACKAGE_TYPE_SYSTEM_BEGIN 0xE0 +#define EFI_HII_PACKAGE_TYPE_SYSTEM_END 0xFF
+struct efi_hii_strings_package {
struct efi_hii_package_header header;uint32_t header_size;uint32_t string_info_offset;uint16_t language_window[16];efi_string_id_t language_name;uint8_t language[];+} __packed;
+struct efi_hii_string_block {
uint8_t block_type;/*uint8_t block_body[];*/+} __packed;
+#define EFI_HII_SIBT_END 0x00 // The end of the string information. +#define EFI_HII_SIBT_STRING_SCSU 0x10 // Single string using default font information. +#define EFI_HII_SIBT_STRING_SCSU_FONT 0x11 // Single string with font information. +#define EFI_HII_SIBT_STRINGS_SCSU 0x12 // Multiple strings using default font information. +#define EFI_HII_SIBT_STRINGS_SCSU_FONT 0x13 // Multiple strings with font information. +#define EFI_HII_SIBT_STRING_UCS2 0x14 // Single UCS-2 string using default font information. +#define EFI_HII_SIBT_STRING_UCS2_FONT 0x15 // Single UCS-2 string with font information +#define EFI_HII_SIBT_STRINGS_UCS2 0x16 // Multiple UCS-2 strings using default font information. +#define EFI_HII_SIBT_STRINGS_UCS2_FONT 0x17 // Multiple UCS-2 strings with font information. +#define EFI_HII_SIBT_DUPLICATE 0x20 // Create a duplicate of an existing string. +#define EFI_HII_SIBT_SKIP2 0x21 // Skip a certain number of string identifiers. +#define EFI_HII_SIBT_SKIP1 0x22 // Skip a certain number of string identifiers. +#define EFI_HII_SIBT_EXT1 0x30 // For future expansion (one byte length field) +#define EFI_HII_SIBT_EXT2 0x31 // For future expansion (two byte length field) +#define EFI_HII_SIBT_EXT4 0x32 // For future expansion (four byte length field) +#define EFI_HII_SIBT_FONT 0x40 // Font information.
+struct efi_hii_sibt_string_ucs2_block {
struct efi_hii_string_block header;uint16_t string_text[];+} __packed;
+static inline struct efi_hii_string_block *efi_hii_sibt_string_ucs2_block_next(
struct efi_hii_sibt_string_ucs2_block *blk)+{
return ((void *)blk) + sizeof(*blk) +(utf16_strlen(blk->string_text) + 1) * 2;Since you're dealing with actual utf16, is this correct? One character may as well span 4 bytes, right?
I think we need a different function that actually tells us the bytes occupied by a utf16 string.
as mentioned on IRC (just repeating here for those who weren't following #u-boot), utf16_strlen() is actually telling us the number of 16b "bytes" in a utf16 string, not the number of "characters". Similar to strlen() with a utf8 string.
+}
+typedef void *efi_hii_handle_t;
+struct efi_hii_database_protocol {
efi_status_t(EFIAPI *new_package_list)(const struct efi_hii_database_protocol *this,const struct efi_hii_package_list_header *package_list,const efi_handle_t driver_handle,efi_hii_handle_t *handle);efi_status_t(EFIAPI *remove_package_list)(const struct efi_hii_database_protocol *this,efi_hii_handle_t handle);efi_status_t(EFIAPI *update_package_list)(const struct efi_hii_database_protocol *this,efi_hii_handle_t handle,const struct efi_hii_package_list_header *package_list);efi_status_t(EFIAPI *list_package_lists)(const struct efi_hii_database_protocol *this,uint8_t package_type,const efi_guid_t *package_guid,efi_uintn_t *handle_buffer_length,efi_hii_handle_t *handle);efi_status_t(EFIAPI *export_package_lists)(const struct efi_hii_database_protocol *this,efi_hii_handle_t handle,efi_uintn_t *buffer_size,struct efi_hii_package_list_header *buffer);efi_status_t(EFIAPI *register_package_notify)(const struct efi_hii_database_protocol *this,uint8_t package_type,const efi_guid_t *package_guid,const void *package_notify_fn,efi_uintn_t notify_type,efi_handle_t *notify_handle);efi_status_t(EFIAPI *unregister_package_notify)(const struct efi_hii_database_protocol *this,efi_handle_t notification_handle);efi_status_t(EFIAPI *find_keyboard_layouts)(const struct efi_hii_database_protocol *this,uint16_t *key_guid_buffer_length,efi_guid_t *key_guid_buffer);efi_status_t(EFIAPI *get_keyboard_layout)(const struct efi_hii_database_protocol *this,efi_guid_t *key_guid,uint16_t *keyboard_layout_length,struct efi_hii_keyboard_layout *keyboard_layout);efi_status_t(EFIAPI *set_keyboard_layout)(const struct efi_hii_database_protocol *this,efi_guid_t *key_guid);efi_status_t(EFIAPI *get_package_list_handle)(const struct efi_hii_database_protocol *this,efi_hii_handle_t package_list_handle,efi_handle_t *driver_handle);+};
+#define EFI_HII_STRING_PROTOCOL_GUID \
EFI_GUID(0x0fd96974, 0x23aa, 0x4cdc, \0xb9, 0xcb, 0x98, 0xd1, 0x77, 0x50, 0x32, 0x2a)+typedef uint32_t efi_hii_font_style_t;
+struct efi_font_info {
efi_hii_font_style_t font_style;uint16_t font_size;uint16_t font_name[1];+};
+struct efi_hii_string_protocol {
efi_status_t(EFIAPI *new_string)(const struct efi_hii_string_protocol *this,efi_hii_handle_t package_list,efi_string_id_t *string_id,const uint8_t *language,const uint16_t *language_name,const efi_string_t string,const struct efi_font_info *string_font_info);efi_status_t(EFIAPI *get_string)(const struct efi_hii_string_protocol *this,const uint8_t *language,efi_hii_handle_t package_list,efi_string_id_t string_id,efi_string_t string,efi_uintn_t *string_size,struct efi_font_info **string_font_info);efi_status_t(EFIAPI *set_string)(const struct efi_hii_string_protocol *this,efi_hii_handle_t package_list,efi_string_id_t string_id,const uint8_t *language,const efi_string_t string,const struct efi_font_info *string_font_info);efi_status_t(EFIAPI *get_languages)(const struct efi_hii_string_protocol *this,efi_hii_handle_t package_list,uint8_t *languages,efi_uintn_t *languages_size);efi_status_t(EFIAPI *get_secondary_languages)(const struct efi_hii_string_protocol *this,efi_hii_handle_t package_list,const uint8_t *primary_language,uint8_t *secondary_languages,efi_uintn_t *secondary_languages_size);+};
#define EFI_GOP_GUID \ EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \ 0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a) diff --git a/include/efi_loader.h b/include/efi_loader.h index 5d37c1d75f..591bf07e7a 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -80,6 +80,9 @@ extern struct efi_simple_input_interface efi_con_in; extern const struct efi_console_control_protocol efi_console_control; extern const struct efi_device_path_to_text_protocol efi_device_path_to_text; extern const struct efi_device_path_utilities_protocol efi_device_path_utilities; +extern const struct efi_hii_config_routing_protocol efi_hii_config_routing; +extern const struct efi_hii_database_protocol efi_hii_database; +extern const struct efi_hii_string_protocol efi_hii_string;
uint16_t *efi_dp_str(struct efi_device_path *dp);
@@ -91,6 +94,9 @@ extern const efi_guid_t efi_guid_device_path_to_text_protocol; extern const efi_guid_t efi_simple_file_system_protocol_guid; extern const efi_guid_t efi_file_info_guid; extern const efi_guid_t efi_guid_device_path_utilities_protocol; +extern const efi_guid_t efi_guid_hii_config_routing_protocol; +extern const efi_guid_t efi_guid_hii_database_protocol; +extern const efi_guid_t efi_guid_hii_string_protocol;
extern unsigned int __efi_runtime_start, __efi_runtime_stop; extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop; diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index b6927b3b84..725e0cba85 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -17,7 +17,7 @@ endif obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o -obj-y += efi_device_path_utilities.o +obj-y += efi_device_path_utilities.o efi_hii.o obj-y += efi_file.o efi_variable.o efi_bootmgr.o obj-$(CONFIG_LCD) += efi_gop.o obj-$(CONFIG_DM_VIDEO) += efi_gop.o diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 92c778fcca..c179afc25a 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1157,6 +1157,15 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob obj->protocols[4].protocol_interface = (void *)&efi_device_path_utilities;
obj->protocols[5].guid = &efi_guid_hii_string_protocol;obj->protocols[5].protocol_interface = (void *)&efi_hii_string;obj->protocols[6].guid = &efi_guid_hii_database_protocol;obj->protocols[6].protocol_interface = (void *)&efi_hii_database;obj->protocols[7].guid = &efi_guid_hii_config_routing_protocol;obj->protocols[7].protocol_interface = (void *)&efi_hii_config_routing;info->file_path = file_path; info->device_handle = efi_dp_find_obj(device_path, NULL);diff --git a/lib/efi_loader/efi_hii.c b/lib/efi_loader/efi_hii.c new file mode 100644 index 0000000000..25c8e88a60 --- /dev/null +++ b/lib/efi_loader/efi_hii.c @@ -0,0 +1,507 @@ +/*
- EFI Human Interface Infrastructure ... interface
- Copyright (c) 2017 Leif Lindholm
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <malloc.h> +#include <efi_loader.h>
+const efi_guid_t efi_guid_hii_config_routing_protocol =
EFI_HII_CONFIG_ROUTING_PROTOCOL_GUID;+const efi_guid_t efi_guid_hii_database_protocol = EFI_HII_DATABASE_PROTOCOL_GUID; +const efi_guid_t efi_guid_hii_string_protocol = EFI_HII_STRING_PROTOCOL_GUID;
+struct hii_package {
// TODO should there be an associated efi_object?struct list_head string_tables; /* list of string_table *//* we could also track fonts, images, etc */+};
+struct string_table {
struct list_head link;efi_string_id_t language_name;char *language;uint32_t nstrings;/* NOTE: string id starts at 1 so value is stbl->strings[id-1] */struct {efi_string_t string;/* we could also track font info, etc */} strings[];+};
+static void free_strings_table(struct string_table *stbl) +{
int i;for (i = 0; i < stbl->nstrings; i++)free(stbl->strings[i].string);free(stbl->language);free(stbl);+}
+static struct hii_package *new_package(void) +{
struct hii_package *hii = malloc(sizeof(*hii));INIT_LIST_HEAD(&hii->string_tables);return hii;+}
+static void free_package(struct hii_package *hii) +{
while (!list_empty(&hii->string_tables)) {struct string_table *stbl;stbl = list_first_entry(&hii->string_tables,struct string_table, link);list_del(&stbl->link);free_strings_table(stbl);}free(hii);+}
+static efi_status_t add_strings_package(struct hii_package *hii,
struct efi_hii_strings_package *strings_package)+{
struct efi_hii_string_block *block;void *end = ((void *)strings_package) + strings_package->header.length;uint32_t nstrings = 0;unsigned id = 0;debug("header_size: %08x\n", strings_package->header_size);debug("string_info_offset: %08x\n", strings_package->string_info_offset);debug("language_name: %u\n", strings_package->language_name);debug("language: %s\n", strings_package->language);/* count # of string entries: */block = ((void *)strings_package) + strings_package->string_info_offset;while ((void *)block < end) {switch (block->block_type) {case EFI_HII_SIBT_STRING_UCS2: {struct efi_hii_sibt_string_ucs2_block *ucs2 =(void *)block;nstrings++;block = efi_hii_sibt_string_ucs2_block_next(ucs2);break;}case EFI_HII_SIBT_END:block = end;break;default:debug("unknown HII string block type: %02x\n",block->block_type);return EFI_INVALID_PARAMETER;}}struct string_table *stbl = malloc(sizeof(*stbl) +(nstrings * sizeof(stbl->strings[0])));stbl->language_name = strings_package->language_name;stbl->language = strdup((char *)strings_package->language);Where does the strings_package come from? And why is the language in it in UTF8?
The strings_package is a part of the "package list" blob passed in from (in this case) Shell.efi. The package list can actually contain a lot more (fonts/glyphs/images/forms), but not really sure how much of that we'll actually support. (HII seems to be able to do enough for rendering a full blown GUI.. kinda overkill, IMHO.. but Shell.efi wants it for silly reasons.)
This is actually utf8.. it is a "RFC 4646 language code identifier". See appendix M.
stbl->nstrings = nstrings;list_add(&stbl->link, &hii->string_tables);/* and now parse string entries and populate string_table */block = ((void *)strings_package) + strings_package->string_info_offset;while ((void *)block < end) {switch (block->block_type) {case EFI_HII_SIBT_STRING_UCS2: {struct efi_hii_sibt_string_ucs2_block *ucs2 =(void *)block;id++;debug("%4u: \"%ls\"\n", id, ucs2->string_text);stbl->strings[id-1].string =utf16_strdup(ucs2->string_text);block = efi_hii_sibt_string_ucs2_block_next(ucs2);break;}case EFI_HII_SIBT_END:return EFI_SUCCESS;default:debug("unknown HII string block type: %02x\n",block->block_type);return EFI_INVALID_PARAMETER;}}return EFI_SUCCESS;+}
+/*
- EFI_HII_CONFIG_ROUTING_PROTOCOL
- */
+static efi_status_t EFIAPI extract_config(
const struct efi_hii_config_routing_protocol *this,const efi_string_t request,efi_string_t *progress,efi_string_t *results)+{
EFI_ENTRY("%p, \"%ls\", %p, %p", this, request, progress, results);return EFI_EXIT(EFI_OUT_OF_RESOURCES);+}
+static efi_status_t EFIAPI export_config(
const struct efi_hii_config_routing_protocol *this,efi_string_t *results)+{
EFI_ENTRY("%p, %p", this, results);return EFI_EXIT(EFI_OUT_OF_RESOURCES);+}
+static efi_status_t EFIAPI route_config(
const struct efi_hii_config_routing_protocol *this,const efi_string_t configuration,efi_string_t *progress)+{
EFI_ENTRY("%p, \"%ls\", %p", this, configuration, progress);return EFI_EXIT(EFI_OUT_OF_RESOURCES);+}
+static efi_status_t EFIAPI block_to_config(
const struct efi_hii_config_routing_protocol *this,const efi_string_t config_request,const uint8_t *block,const efi_uintn_t block_size,efi_string_t *config,efi_string_t *progress)+{
EFI_ENTRY("%p, \"%ls\", %p, %zu, %p, %p", this, config_request, block,block_size, config, progress);return EFI_EXIT(EFI_OUT_OF_RESOURCES);+}
+static efi_status_t EFIAPI config_to_block(
const struct efi_hii_config_routing_protocol *this,const efi_string_t config_resp,const uint8_t *block,const efi_uintn_t *block_size,efi_string_t *progress)+{
EFI_ENTRY("%p, \"%ls\", %p, %p, %p", this, config_resp, block,block_size, progress);return EFI_EXIT(EFI_OUT_OF_RESOURCES);+}
+static efi_status_t EFIAPI get_alt_config(
const struct efi_hii_config_routing_protocol *this,const efi_string_t config_resp,const efi_guid_t *guid,const efi_string_t name,const struct efi_device_path *device_path,const efi_string_t alt_cfg_id,efi_string_t *alt_cfg_resp)+{
EFI_ENTRY("%p, \"%ls\", %pUl, \"%ls\", %p, \"%ls\", %p", this,config_resp, guid, name, device_path, alt_cfg_id,alt_cfg_resp);return EFI_EXIT(EFI_OUT_OF_RESOURCES);+}
+/*
- EFI_HII_DATABASE_PROTOCOL
- */
+static efi_status_t EFIAPI new_package_list(
const struct efi_hii_database_protocol *this,const struct efi_hii_package_list_header *package_list,const efi_handle_t driver_handle,efi_hii_handle_t *handle)+{
efi_status_t ret = EFI_SUCCESS;EFI_ENTRY("%p, %p, %p, %p", this, package_list, driver_handle, handle);if (!package_list || !driver_handle)return EFI_EXIT(EFI_INVALID_PARAMETER);struct hii_package *hii = new_package();struct efi_hii_package_header *package;void *end = ((void *)package_list) + package_list->package_length;debug("package_list: %pUl (%u)\n", &package_list->package_list_guid,package_list->package_length);package = ((void *)package_list) + sizeof(*package_list);while ((void *)package < end) {debug("package=%p, package type=%x, length=%u\n", package,package->type, package->length);switch (package->type) {case EFI_HII_PACKAGE_STRINGS:ret = add_strings_package(hii,(struct efi_hii_strings_package *)package);break;default:break;}if (ret != EFI_SUCCESS)goto error;package = ((void *)package) + package->length;}// TODO in theory there is some notifications that should be sent..*handle = hii;return EFI_EXIT(EFI_SUCCESS);+error:
free_package(hii);return EFI_EXIT(ret);+}
+static efi_status_t EFIAPI remove_package_list(
const struct efi_hii_database_protocol *this,efi_hii_handle_t handle)+{
struct hii_package *hii = handle;EFI_ENTRY("%p, %p", this, handle);free_package(hii);return EFI_EXIT(EFI_SUCCESS);+}
+static efi_status_t EFIAPI update_package_list(
const struct efi_hii_database_protocol *this,efi_hii_handle_t handle,const struct efi_hii_package_list_header *package_list)+{
EFI_ENTRY("%p, %p, %p", this, handle, package_list);return EFI_EXIT(EFI_NOT_FOUND);+}
+static efi_status_t EFIAPI list_package_lists(
const struct efi_hii_database_protocol *this,uint8_t package_type,const efi_guid_t *package_guid,efi_uintn_t *handle_buffer_length,efi_hii_handle_t *handle)+{
EFI_ENTRY("%p, %u, %pUl, %p, %p", this, package_type, package_guid,handle_buffer_length, handle);return EFI_EXIT(EFI_NOT_FOUND);+}
+static efi_status_t EFIAPI export_package_lists(
const struct efi_hii_database_protocol *this,efi_hii_handle_t handle,efi_uintn_t *buffer_size,struct efi_hii_package_list_header *buffer)+{
EFI_ENTRY("%p, %p, %p, %p", this, handle, buffer_size, buffer);return EFI_EXIT(EFI_NOT_FOUND);+}
+static efi_status_t EFIAPI register_package_notify(
const struct efi_hii_database_protocol *this,uint8_t package_type,const efi_guid_t *package_guid,const void *package_notify_fn,efi_uintn_t notify_type,efi_handle_t *notify_handle)+{
EFI_ENTRY("%p, %u, %pUl, %p, %zu, %p", this, package_type,package_guid, package_notify_fn, notify_type,notify_handle);return EFI_EXIT(EFI_OUT_OF_RESOURCES);+}
+static efi_status_t EFIAPI unregister_package_notify(
const struct efi_hii_database_protocol *this,efi_handle_t notification_handle)+{
EFI_ENTRY("%p, %p", this, notification_handle);return EFI_EXIT(EFI_NOT_FOUND);+}
+static efi_status_t EFIAPI find_keyboard_layouts(
const struct efi_hii_database_protocol *this,uint16_t *key_guid_buffer_length,efi_guid_t *key_guid_buffer)+{
EFI_ENTRY("%p, %p, %p", this, key_guid_buffer_length, key_guid_buffer);return EFI_EXIT(EFI_NOT_FOUND); /* Invalid */+}
+static efi_status_t EFIAPI get_keyboard_layout(
const struct efi_hii_database_protocol *this,efi_guid_t *key_guid,uint16_t *keyboard_layout_length,struct efi_hii_keyboard_layout *keyboard_layout)+{
EFI_ENTRY("%p, %pUl, %p, %p", this, key_guid, keyboard_layout_length,keyboard_layout);return EFI_EXIT(EFI_NOT_FOUND);+}
+static efi_status_t EFIAPI set_keyboard_layout(
const struct efi_hii_database_protocol *this,efi_guid_t *key_guid)+{
EFI_ENTRY("%p, %pUl", this, key_guid);return EFI_EXIT(EFI_NOT_FOUND);+}
+static efi_status_t EFIAPI get_package_list_handle(
const struct efi_hii_database_protocol *this,efi_hii_handle_t package_list_handle,efi_handle_t *driver_handle)+{
EFI_ENTRY("%p, %p, %p", this, package_list_handle, driver_handle);return EFI_EXIT(EFI_INVALID_PARAMETER);+}
+/*
- EFI_HII_STRING_PROTOCOL
- */
+static efi_status_t EFIAPI new_string(
const struct efi_hii_string_protocol *this,efi_hii_handle_t package_list,efi_string_id_t *string_id,const uint8_t *language,const uint16_t *language_name,const efi_string_t string,const struct efi_font_info *string_font_info)+{
EFI_ENTRY("%p, %p, %p, \"%s\", %p, \"%ls\", %p", this, package_list,string_id, language, language_name, string,string_font_info);return EFI_EXIT(EFI_NOT_FOUND);+}
+static efi_status_t EFIAPI get_string(
const struct efi_hii_string_protocol *this,const uint8_t *language,efi_hii_handle_t package_list,efi_string_id_t string_id,efi_string_t string,efi_uintn_t *string_size,struct efi_font_info **string_font_info)+{
struct hii_package *hii = package_list;struct string_table *stbl;EFI_ENTRY("%p, \"%s\", %p, %u, %p, %p, %p", this, language,package_list, string_id, string, string_size,string_font_info);list_for_each_entry(stbl, &hii->string_tables, link) {if (!strcmp((char *)language, (char *)stbl->language)) {unsigned idx = string_id - 1;if (idx > stbl->nstrings)return EFI_EXIT(EFI_NOT_FOUND);efi_string_t str = stbl->strings[idx].string;size_t len = utf16_strlen(str) + 1;I assume that's wrong for sizing too?
nope :-)
Also please try not to define variables mid-scope :). Just define them above the if() and fill them after ...
Hmm, I'd have the inverse comment if someone else wrote it and defined all the variables at the top ;-)
Then also for the sake of readability add a blank line after the variable definitions and before the return.
I think you'd also do yourself a favor if you reduced the indenting a bit. Just abort the list_for_each when you found and entry and then process it in the top level scope.
static struct string_table *find_stbl_by_lang(const char *language) { list_for_each(...) { if (matches) { return stbl; } }
return NULL;}
stbl = find_stbl_by_lang(lang); if (!stbl) return EFI_EXIT(EFI_NOT_FOUND)
yeah, perhaps
Please rework it for v2 :)
Alex