Re: [U-Boot] [PATCH 1/2] efi_loader: implement GetNextVariableName()
On Thu, Jan 17, 2019 at 08:09:57AM +0100, Heinrich Schuchardt wrote: > On 1/17/19 3:14 AM, AKASHI Takahiro wrote: > > Heinrich, > > > > Thank you for your quick review. > > > > On Wed, Jan 16, 2019 at 07:43:47PM +0100, Heinrich Schuchardt wrote: > >> On 12/14/18 10:47 AM, AKASHI Takahiro wrote: > >>> The current GetNextVariableName() is a placeholder. > >>> With this patch, it works well as expected. > >>> > >>> Signed-off-by: AKASHI Takahiro > >>> --- > >>> lib/efi_loader/efi_variable.c | 116 +- > >>> 1 file changed, 115 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > >>> index 19d9cb865f25..dac2f49aa1cc 100644 > >>> --- a/lib/efi_loader/efi_variable.c > >>> +++ b/lib/efi_loader/efi_variable.c > >>> @@ -8,6 +8,9 @@ > >>> #include > >>> #include > >>> #include > >>> +#include > >>> +#include > >>> +#include > >>> > >>> #define READ_ONLY BIT(31) > >>> > >>> @@ -241,14 +244,125 @@ efi_status_t EFIAPI efi_get_variable(u16 > >>> *variable_name, efi_guid_t *vendor, > >>> return EFI_EXIT(EFI_SUCCESS); > >>> } > >>> > >>> +static char *efi_variables_list; > >>> +static char *efi_cur_variable; > >>> + > >> > >> Please, provide a comment describing what this function is meant to do. > > > > I think that the function name describes itself clearly, but OK. > > > >> Describe every parameter > >> > >> Clearly state set variable_name_size is in bytes (cf. > >> EmuGetNextVariableName() in EDK2) > > > > Right. > > > >> This function duplicates some of the code in efi_get_variable. So, > >> please, use it in efi_get_variable too. > > > > Which part of code do you mean? I don't think so. > > Checking buffer size. > Comparing vendor GUID. > Extracting an EFI variable from a U-Boot variable. Please specify exact line numbers on both side. Otherwise, I don't get you. > > > >>> +static efi_status_t parse_uboot_variable(char *variable, > >>> + efi_uintn_t *variable_name_size, > >>> + u16 *variable_name, > >>> + efi_guid_t *vendor, > >>> + u32 *attribute) > >>> +{ > >> > >> > >> > >>> + char *guid, *name, *end, c; > >>> + unsigned long name_size; > >>> + u16 *p; > >>> + > >>> + guid = strchr(variable, '_'); > >>> + if (!guid) > >>> + return EFI_NOT_FOUND; > >>> + guid++; > >>> + name = strchr(guid, '_'); > >>> + if (!name) > >>> + return EFI_NOT_FOUND; > >>> + name++; > >>> + end = strchr(name, '='); > >>> + if (!end) > >>> + return EFI_NOT_FOUND; > >>> + > >>> + /* FIXME: size is in byte or u16? */ > >> > >> It is in bytes. See comment above. > > > > OK > > > >>> + name_size = end - name; > >>> + if (*variable_name_size < (name_size + 1)) { > >>> + *variable_name_size = name_size + 1; > >>> + return EFI_BUFFER_TOO_SMALL; > >>> + } > >>> + end++; /* point to value */ > >>> + > >>> + p = variable_name; > >>> + utf8_utf16_strncpy(, name, name_size); > >>> + variable_name[name_size] = 0; > >>> + > >>> + c = *(name - 1); > >>> + *(name - 1) = '\0'; /* guid need be null-terminated here */ > >>> + uuid_str_to_bin(guid, (unsigned char *)vendor, UUID_STR_FORMAT_GUID); > >>> + *(name - 1) = c; > >>> + > >>> + parse_attr(end, attribute); > >> > >> You have to update variable_name_size. > > > > Right. > > > >>> + > >>> + return EFI_SUCCESS; > >>> +} > >>> + > >>> /* > >>> http://wiki.phoenix.com/wiki/index.php/EFI_RUNTIME_SERVICES#GetNextVariableName.28.29 > >>> */ > >> > >> Please add a description of the function here like we have it in > >> efi_bootefi.c > > > > OK, but not for efi_get/set_variable() as I didn't touch anything there. > > For the other functions we should do it in a separate patch. Not me. > > > >>> efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t > >>> *variable_name_size, > >>> u16 *variable_name, > >>> efi_guid_t *vendor) > >>> { > >>> + char *native_name, *variable; > >>> + ssize_t name_len, list_len; > >>> + char regex[256]; > >>> + char * const regexlist[] = {regex}; > >>> + u32 attribute; > >>> + int i; > >>> + efi_status_t ret; > >>> + > >>> EFI_ENTRY("%p \"%ls\" %pUl", variable_name_size, variable_name, vendor); > >>> > >>> - return EFI_EXIT(EFI_DEVICE_ERROR); > >>> + if (!variable_name_size || !variable_name || !vendor) > >>> + EFI_EXIT(EFI_INVALID_PARAMETER); > >>> + > >>> + if (variable_name[0]) { > >> > >> This code partially duplicates code in in efi_get_variable. Please, > >> carve out a common function. > > > > Which part of code do you mean? I don't see any duplication. What about this? > > > >>> + /* check null-terminated string */ > >>> + for (i = 0; i < *variable_name_size; i++) > >>> + if (!variable_name[i]) > >>> + break;
Re: [U-Boot] [PATCH 1/2] efi_loader: implement GetNextVariableName()
On 1/17/19 3:14 AM, AKASHI Takahiro wrote: > Heinrich, > > Thank you for your quick review. > > On Wed, Jan 16, 2019 at 07:43:47PM +0100, Heinrich Schuchardt wrote: >> On 12/14/18 10:47 AM, AKASHI Takahiro wrote: >>> The current GetNextVariableName() is a placeholder. >>> With this patch, it works well as expected. >>> >>> Signed-off-by: AKASHI Takahiro >>> --- >>> lib/efi_loader/efi_variable.c | 116 +- >>> 1 file changed, 115 insertions(+), 1 deletion(-) >>> >>> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c >>> index 19d9cb865f25..dac2f49aa1cc 100644 >>> --- a/lib/efi_loader/efi_variable.c >>> +++ b/lib/efi_loader/efi_variable.c >>> @@ -8,6 +8,9 @@ >>> #include >>> #include >>> #include >>> +#include >>> +#include >>> +#include >>> >>> #define READ_ONLY BIT(31) >>> >>> @@ -241,14 +244,125 @@ efi_status_t EFIAPI efi_get_variable(u16 >>> *variable_name, efi_guid_t *vendor, >>> return EFI_EXIT(EFI_SUCCESS); >>> } >>> >>> +static char *efi_variables_list; >>> +static char *efi_cur_variable; >>> + >> >> Please, provide a comment describing what this function is meant to do. > > I think that the function name describes itself clearly, but OK. > >> Describe every parameter >> >> Clearly state set variable_name_size is in bytes (cf. >> EmuGetNextVariableName() in EDK2) > > Right. > >> This function duplicates some of the code in efi_get_variable. So, >> please, use it in efi_get_variable too. > > Which part of code do you mean? I don't think so. Checking buffer size. Comparing vendor GUID. Extracting an EFI variable from a U-Boot variable. > >>> +static efi_status_t parse_uboot_variable(char *variable, >>> +efi_uintn_t *variable_name_size, >>> +u16 *variable_name, >>> +efi_guid_t *vendor, >>> +u32 *attribute) >>> +{ >> >> >> >>> + char *guid, *name, *end, c; >>> + unsigned long name_size; >>> + u16 *p; >>> + >>> + guid = strchr(variable, '_'); >>> + if (!guid) >>> + return EFI_NOT_FOUND; >>> + guid++; >>> + name = strchr(guid, '_'); >>> + if (!name) >>> + return EFI_NOT_FOUND; >>> + name++; >>> + end = strchr(name, '='); >>> + if (!end) >>> + return EFI_NOT_FOUND; >>> + >>> + /* FIXME: size is in byte or u16? */ >> >> It is in bytes. See comment above. > > OK > >>> + name_size = end - name; >>> + if (*variable_name_size < (name_size + 1)) { >>> + *variable_name_size = name_size + 1; >>> + return EFI_BUFFER_TOO_SMALL; >>> + } >>> + end++; /* point to value */ >>> + >>> + p = variable_name; >>> + utf8_utf16_strncpy(, name, name_size); >>> + variable_name[name_size] = 0; >>> + >>> + c = *(name - 1); >>> + *(name - 1) = '\0'; /* guid need be null-terminated here */ >>> + uuid_str_to_bin(guid, (unsigned char *)vendor, UUID_STR_FORMAT_GUID); >>> + *(name - 1) = c; >>> + >>> + parse_attr(end, attribute); >> >> You have to update variable_name_size. > > Right. > >>> + >>> + return EFI_SUCCESS; >>> +} >>> + >>> /* >>> http://wiki.phoenix.com/wiki/index.php/EFI_RUNTIME_SERVICES#GetNextVariableName.28.29 >>> */ >> >> Please add a description of the function here like we have it in >> efi_bootefi.c > > OK, but not for efi_get/set_variable() as I didn't touch anything there. For the other functions we should do it in a separate patch. > >>> efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t >>> *variable_name_size, >>>u16 *variable_name, >>>efi_guid_t *vendor) >>> { >>> + char *native_name, *variable; >>> + ssize_t name_len, list_len; >>> + char regex[256]; >>> + char * const regexlist[] = {regex}; >>> + u32 attribute; >>> + int i; >>> + efi_status_t ret; >>> + >>> EFI_ENTRY("%p \"%ls\" %pUl", variable_name_size, variable_name, vendor); >>> >>> - return EFI_EXIT(EFI_DEVICE_ERROR); >>> + if (!variable_name_size || !variable_name || !vendor) >>> + EFI_EXIT(EFI_INVALID_PARAMETER); >>> + >>> + if (variable_name[0]) { >> >> This code partially duplicates code in in efi_get_variable. Please, >> carve out a common function. > > Which part of code do you mean? I don't see any duplication. > >>> + /* check null-terminated string */ >>> + for (i = 0; i < *variable_name_size; i++) >>> + if (!variable_name[i]) >>> + break; >>> + if (i >= *variable_name_size) >>> + return EFI_EXIT(EFI_INVALID_PARAMETER); >>> + >>> + /* search for the last-returned variable */ >>> + ret = efi_to_native(_name, variable_name, vendor); >>> + if (ret) >>> + return EFI_EXIT(ret); >>> + >>> + name_len = strlen(native_name); >>> +
Re: [U-Boot] [PATCH 1/2] efi_loader: implement GetNextVariableName()
Heinrich, Thank you for your quick review. On Wed, Jan 16, 2019 at 07:43:47PM +0100, Heinrich Schuchardt wrote: > On 12/14/18 10:47 AM, AKASHI Takahiro wrote: > > The current GetNextVariableName() is a placeholder. > > With this patch, it works well as expected. > > > > Signed-off-by: AKASHI Takahiro > > --- > > lib/efi_loader/efi_variable.c | 116 +- > > 1 file changed, 115 insertions(+), 1 deletion(-) > > > > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > > index 19d9cb865f25..dac2f49aa1cc 100644 > > --- a/lib/efi_loader/efi_variable.c > > +++ b/lib/efi_loader/efi_variable.c > > @@ -8,6 +8,9 @@ > > #include > > #include > > #include > > +#include > > +#include > > +#include > > > > #define READ_ONLY BIT(31) > > > > @@ -241,14 +244,125 @@ efi_status_t EFIAPI efi_get_variable(u16 > > *variable_name, efi_guid_t *vendor, > > return EFI_EXIT(EFI_SUCCESS); > > } > > > > +static char *efi_variables_list; > > +static char *efi_cur_variable; > > + > > Please, provide a comment describing what this function is meant to do. I think that the function name describes itself clearly, but OK. > Describe every parameter > > Clearly state set variable_name_size is in bytes (cf. > EmuGetNextVariableName() in EDK2) Right. > This function duplicates some of the code in efi_get_variable. So, > please, use it in efi_get_variable too. Which part of code do you mean? I don't think so. > > +static efi_status_t parse_uboot_variable(char *variable, > > +efi_uintn_t *variable_name_size, > > +u16 *variable_name, > > +efi_guid_t *vendor, > > +u32 *attribute) > > +{ > > > > > + char *guid, *name, *end, c; > > + unsigned long name_size; > > + u16 *p; > > + > > + guid = strchr(variable, '_'); > > + if (!guid) > > + return EFI_NOT_FOUND; > > + guid++; > > + name = strchr(guid, '_'); > > + if (!name) > > + return EFI_NOT_FOUND; > > + name++; > > + end = strchr(name, '='); > > + if (!end) > > + return EFI_NOT_FOUND; > > + > > + /* FIXME: size is in byte or u16? */ > > It is in bytes. See comment above. OK > > + name_size = end - name; > > + if (*variable_name_size < (name_size + 1)) { > > + *variable_name_size = name_size + 1; > > + return EFI_BUFFER_TOO_SMALL; > > + } > > + end++; /* point to value */ > > + > > + p = variable_name; > > + utf8_utf16_strncpy(, name, name_size); > > + variable_name[name_size] = 0; > > + > > + c = *(name - 1); > > + *(name - 1) = '\0'; /* guid need be null-terminated here */ > > + uuid_str_to_bin(guid, (unsigned char *)vendor, UUID_STR_FORMAT_GUID); > > + *(name - 1) = c; > > + > > + parse_attr(end, attribute); > > You have to update variable_name_size. Right. > > + > > + return EFI_SUCCESS; > > +} > > + > > /* > > http://wiki.phoenix.com/wiki/index.php/EFI_RUNTIME_SERVICES#GetNextVariableName.28.29 > > */ > > Please add a description of the function here like we have it in > efi_bootefi.c OK, but not for efi_get/set_variable() as I didn't touch anything there. > > efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t > > *variable_name_size, > >u16 *variable_name, > >efi_guid_t *vendor) > > { > > + char *native_name, *variable; > > + ssize_t name_len, list_len; > > + char regex[256]; > > + char * const regexlist[] = {regex}; > > + u32 attribute; > > + int i; > > + efi_status_t ret; > > + > > EFI_ENTRY("%p \"%ls\" %pUl", variable_name_size, variable_name, vendor); > > > > - return EFI_EXIT(EFI_DEVICE_ERROR); > > + if (!variable_name_size || !variable_name || !vendor) > > + EFI_EXIT(EFI_INVALID_PARAMETER); > > + > > + if (variable_name[0]) { > > This code partially duplicates code in in efi_get_variable. Please, > carve out a common function. Which part of code do you mean? I don't see any duplication. > > + /* check null-terminated string */ > > + for (i = 0; i < *variable_name_size; i++) > > + if (!variable_name[i]) > > + break; > > + if (i >= *variable_name_size) > > + return EFI_EXIT(EFI_INVALID_PARAMETER); > > + > > + /* search for the last-returned variable */ > > + ret = efi_to_native(_name, variable_name, vendor); > > + if (ret) > > + return EFI_EXIT(ret); > > + > > + name_len = strlen(native_name); > > + for (variable = efi_variables_list; variable && *variable;) { > > + if (!strncmp(variable, native_name, name_len) && > > + variable[name_len] == '=') > > + break; > > + > > You miss to compare
Re: [U-Boot] [PATCH 1/2] efi_loader: implement GetNextVariableName()
On 12/14/18 10:47 AM, AKASHI Takahiro wrote: > The current GetNextVariableName() is a placeholder. > With this patch, it works well as expected. > > Signed-off-by: AKASHI Takahiro > --- > lib/efi_loader/efi_variable.c | 116 +- > 1 file changed, 115 insertions(+), 1 deletion(-) > > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > index 19d9cb865f25..dac2f49aa1cc 100644 > --- a/lib/efi_loader/efi_variable.c > +++ b/lib/efi_loader/efi_variable.c > @@ -8,6 +8,9 @@ > #include > #include > #include > +#include > +#include > +#include > > #define READ_ONLY BIT(31) > > @@ -241,14 +244,125 @@ efi_status_t EFIAPI efi_get_variable(u16 > *variable_name, efi_guid_t *vendor, > return EFI_EXIT(EFI_SUCCESS); > } > > +static char *efi_variables_list; > +static char *efi_cur_variable; > + Please, provide a comment describing what this function is meant to do. Describe every parameter Clearly state set variable_name_size is in bytes (cf. EmuGetNextVariableName() in EDK2) This function duplicates some of the code in efi_get_variable. So, please, use it in efi_get_variable too. > +static efi_status_t parse_uboot_variable(char *variable, > + efi_uintn_t *variable_name_size, > + u16 *variable_name, > + efi_guid_t *vendor, > + u32 *attribute) > +{ > + char *guid, *name, *end, c; > + unsigned long name_size; > + u16 *p; > + > + guid = strchr(variable, '_'); > + if (!guid) > + return EFI_NOT_FOUND; > + guid++; > + name = strchr(guid, '_'); > + if (!name) > + return EFI_NOT_FOUND; > + name++; > + end = strchr(name, '='); > + if (!end) > + return EFI_NOT_FOUND; > + > + /* FIXME: size is in byte or u16? */ It is in bytes. See comment above. > + name_size = end - name; > + if (*variable_name_size < (name_size + 1)) { > + *variable_name_size = name_size + 1; > + return EFI_BUFFER_TOO_SMALL; > + } > + end++; /* point to value */ > + > + p = variable_name; > + utf8_utf16_strncpy(, name, name_size); > + variable_name[name_size] = 0; > + > + c = *(name - 1); > + *(name - 1) = '\0'; /* guid need be null-terminated here */ > + uuid_str_to_bin(guid, (unsigned char *)vendor, UUID_STR_FORMAT_GUID); > + *(name - 1) = c; > + > + parse_attr(end, attribute); You have to update variable_name_size. > + > + return EFI_SUCCESS; > +} > + > /* > http://wiki.phoenix.com/wiki/index.php/EFI_RUNTIME_SERVICES#GetNextVariableName.28.29 > */ Please add a description of the function here like we have it in efi_bootefi.c > efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t > *variable_name_size, > u16 *variable_name, > efi_guid_t *vendor) > { > + char *native_name, *variable; > + ssize_t name_len, list_len; > + char regex[256]; > + char * const regexlist[] = {regex}; > + u32 attribute; > + int i; > + efi_status_t ret; > + > EFI_ENTRY("%p \"%ls\" %pUl", variable_name_size, variable_name, vendor); > > - return EFI_EXIT(EFI_DEVICE_ERROR); > + if (!variable_name_size || !variable_name || !vendor) > + EFI_EXIT(EFI_INVALID_PARAMETER); > + > + if (variable_name[0]) { This code partially duplicates code in in efi_get_variable. Please, carve out a common function. > + /* check null-terminated string */ > + for (i = 0; i < *variable_name_size; i++) > + if (!variable_name[i]) > + break; > + if (i >= *variable_name_size) > + return EFI_EXIT(EFI_INVALID_PARAMETER); > + > + /* search for the last-returned variable */ > + ret = efi_to_native(_name, variable_name, vendor); > + if (ret) > + return EFI_EXIT(ret); > + > + name_len = strlen(native_name); > + for (variable = efi_variables_list; variable && *variable;) { > + if (!strncmp(variable, native_name, name_len) && > + variable[name_len] == '=') > + break; > + You miss to compare the GUID. Consider the case that the GUID changes between two calls. > + variable = strchr(variable, '\n'); > + if (variable) > + variable++; > + } > + > + free(native_name); > + if (!(variable && *variable)) With less parentheses I can read the logic more easily: if (!variable || !*variable) But that is just a question of taste. Please, consider the case that the variable is not on the list because the variable has already
Re: [U-Boot] [PATCH 1/2] efi_loader: implement GetNextVariableName()
Alex, Heinrich, Please review this patch set. Thanks, -Takahiro Akashi On Fri, Dec 14, 2018 at 06:47:11PM +0900, AKASHI Takahiro wrote: > The current GetNextVariableName() is a placeholder. > With this patch, it works well as expected. > > Signed-off-by: AKASHI Takahiro > --- > lib/efi_loader/efi_variable.c | 116 +- > 1 file changed, 115 insertions(+), 1 deletion(-) > > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > index 19d9cb865f25..dac2f49aa1cc 100644 > --- a/lib/efi_loader/efi_variable.c > +++ b/lib/efi_loader/efi_variable.c > @@ -8,6 +8,9 @@ > #include > #include > #include > +#include > +#include > +#include > > #define READ_ONLY BIT(31) > > @@ -241,14 +244,125 @@ efi_status_t EFIAPI efi_get_variable(u16 > *variable_name, efi_guid_t *vendor, > return EFI_EXIT(EFI_SUCCESS); > } > > +static char *efi_variables_list; > +static char *efi_cur_variable; > + > +static efi_status_t parse_uboot_variable(char *variable, > + efi_uintn_t *variable_name_size, > + u16 *variable_name, > + efi_guid_t *vendor, > + u32 *attribute) > +{ > + char *guid, *name, *end, c; > + unsigned long name_size; > + u16 *p; > + > + guid = strchr(variable, '_'); > + if (!guid) > + return EFI_NOT_FOUND; > + guid++; > + name = strchr(guid, '_'); > + if (!name) > + return EFI_NOT_FOUND; > + name++; > + end = strchr(name, '='); > + if (!end) > + return EFI_NOT_FOUND; > + > + /* FIXME: size is in byte or u16? */ > + name_size = end - name; > + if (*variable_name_size < (name_size + 1)) { > + *variable_name_size = name_size + 1; > + return EFI_BUFFER_TOO_SMALL; > + } > + end++; /* point to value */ > + > + p = variable_name; > + utf8_utf16_strncpy(, name, name_size); > + variable_name[name_size] = 0; > + > + c = *(name - 1); > + *(name - 1) = '\0'; /* guid need be null-terminated here */ > + uuid_str_to_bin(guid, (unsigned char *)vendor, UUID_STR_FORMAT_GUID); > + *(name - 1) = c; > + > + parse_attr(end, attribute); > + > + return EFI_SUCCESS; > +} > + > /* > http://wiki.phoenix.com/wiki/index.php/EFI_RUNTIME_SERVICES#GetNextVariableName.28.29 > */ > efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t > *variable_name_size, > u16 *variable_name, > efi_guid_t *vendor) > { > + char *native_name, *variable; > + ssize_t name_len, list_len; > + char regex[256]; > + char * const regexlist[] = {regex}; > + u32 attribute; > + int i; > + efi_status_t ret; > + > EFI_ENTRY("%p \"%ls\" %pUl", variable_name_size, variable_name, vendor); > > - return EFI_EXIT(EFI_DEVICE_ERROR); > + if (!variable_name_size || !variable_name || !vendor) > + EFI_EXIT(EFI_INVALID_PARAMETER); > + > + if (variable_name[0]) { > + /* check null-terminated string */ > + for (i = 0; i < *variable_name_size; i++) > + if (!variable_name[i]) > + break; > + if (i >= *variable_name_size) > + return EFI_EXIT(EFI_INVALID_PARAMETER); > + > + /* search for the last-returned variable */ > + ret = efi_to_native(_name, variable_name, vendor); > + if (ret) > + return EFI_EXIT(ret); > + > + name_len = strlen(native_name); > + for (variable = efi_variables_list; variable && *variable;) { > + if (!strncmp(variable, native_name, name_len) && > + variable[name_len] == '=') > + break; > + > + variable = strchr(variable, '\n'); > + if (variable) > + variable++; > + } > + > + free(native_name); > + if (!(variable && *variable)) > + return EFI_EXIT(EFI_INVALID_PARAMETER); > + > + /* next variable */ > + variable = strchr(variable, '\n'); > + if (variable) > + variable++; > + if (!(variable && *variable)) > + return EFI_EXIT(EFI_NOT_FOUND); > + } else { > + /* new search */ > + free(efi_variables_list); > + efi_variables_list = NULL; > + efi_cur_variable = NULL; > + > + snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*"); > + list_len = hexport_r(_htab, '\n', > + H_MATCH_REGEX | H_MATCH_KEY, > + _variables_list, 0, 1, regexlist); > + if
[U-Boot] [PATCH 1/2] efi_loader: implement GetNextVariableName()
The current GetNextVariableName() is a placeholder. With this patch, it works well as expected. Signed-off-by: AKASHI Takahiro --- lib/efi_loader/efi_variable.c | 116 +- 1 file changed, 115 insertions(+), 1 deletion(-) diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 19d9cb865f25..dac2f49aa1cc 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -8,6 +8,9 @@ #include #include #include +#include +#include +#include #define READ_ONLY BIT(31) @@ -241,14 +244,125 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name, efi_guid_t *vendor, return EFI_EXIT(EFI_SUCCESS); } +static char *efi_variables_list; +static char *efi_cur_variable; + +static efi_status_t parse_uboot_variable(char *variable, +efi_uintn_t *variable_name_size, +u16 *variable_name, +efi_guid_t *vendor, +u32 *attribute) +{ + char *guid, *name, *end, c; + unsigned long name_size; + u16 *p; + + guid = strchr(variable, '_'); + if (!guid) + return EFI_NOT_FOUND; + guid++; + name = strchr(guid, '_'); + if (!name) + return EFI_NOT_FOUND; + name++; + end = strchr(name, '='); + if (!end) + return EFI_NOT_FOUND; + + /* FIXME: size is in byte or u16? */ + name_size = end - name; + if (*variable_name_size < (name_size + 1)) { + *variable_name_size = name_size + 1; + return EFI_BUFFER_TOO_SMALL; + } + end++; /* point to value */ + + p = variable_name; + utf8_utf16_strncpy(, name, name_size); + variable_name[name_size] = 0; + + c = *(name - 1); + *(name - 1) = '\0'; /* guid need be null-terminated here */ + uuid_str_to_bin(guid, (unsigned char *)vendor, UUID_STR_FORMAT_GUID); + *(name - 1) = c; + + parse_attr(end, attribute); + + return EFI_SUCCESS; +} + /* http://wiki.phoenix.com/wiki/index.php/EFI_RUNTIME_SERVICES#GetNextVariableName.28.29 */ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, u16 *variable_name, efi_guid_t *vendor) { + char *native_name, *variable; + ssize_t name_len, list_len; + char regex[256]; + char * const regexlist[] = {regex}; + u32 attribute; + int i; + efi_status_t ret; + EFI_ENTRY("%p \"%ls\" %pUl", variable_name_size, variable_name, vendor); - return EFI_EXIT(EFI_DEVICE_ERROR); + if (!variable_name_size || !variable_name || !vendor) + EFI_EXIT(EFI_INVALID_PARAMETER); + + if (variable_name[0]) { + /* check null-terminated string */ + for (i = 0; i < *variable_name_size; i++) + if (!variable_name[i]) + break; + if (i >= *variable_name_size) + return EFI_EXIT(EFI_INVALID_PARAMETER); + + /* search for the last-returned variable */ + ret = efi_to_native(_name, variable_name, vendor); + if (ret) + return EFI_EXIT(ret); + + name_len = strlen(native_name); + for (variable = efi_variables_list; variable && *variable;) { + if (!strncmp(variable, native_name, name_len) && + variable[name_len] == '=') + break; + + variable = strchr(variable, '\n'); + if (variable) + variable++; + } + + free(native_name); + if (!(variable && *variable)) + return EFI_EXIT(EFI_INVALID_PARAMETER); + + /* next variable */ + variable = strchr(variable, '\n'); + if (variable) + variable++; + if (!(variable && *variable)) + return EFI_EXIT(EFI_NOT_FOUND); + } else { + /* new search */ + free(efi_variables_list); + efi_variables_list = NULL; + efi_cur_variable = NULL; + + snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*"); + list_len = hexport_r(_htab, '\n', +H_MATCH_REGEX | H_MATCH_KEY, +_variables_list, 0, 1, regexlist); + if (list_len <= 0) + return EFI_EXIT(EFI_NOT_FOUND); + + variable = efi_variables_list; + } + + ret = parse_uboot_variable(variable, variable_name_size, variable_name, + vendor,