Re: [PATCH v2 1/1] efi_loader: fix HII GetString()

["Vincent Stehlé" <vincent.stehle@arm.com>]

On Mon, Oct 12, 2026 at 09:10:53PM +0200, Heinrich Schuchardt wrote:
> * Add missing conformance tests.

Hi Heinrich,

Thank you for continuing the work on the HII protocols.
I do not see those additional tests in this patch.

Also, one comment below.

Best regards,
Vincent.

> * Return EFI_INVALID_LANGUAGE if a string does not exist in the requested
>   language.
> * Handle querying the string size with *string_size == 0 and string = NULL.
> 
> Fixes: c9bfb22296b0 ("efi_loader: Initial HII database protocols")
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v2:
> 	string_id == 0 is not used by our code. Return EFI_NOT_FOUND.
> ---
>  lib/efi_loader/efi_hii.c | 34 ++++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_hii.c b/lib/efi_loader/efi_hii.c
> index 90dff91e5d4..dc02d7d2ba6 100644
> --- a/lib/efi_loader/efi_hii.c
> +++ b/lib/efi_loader/efi_hii.c
> @@ -910,41 +910,55 @@ get_string(const struct efi_hii_string_protocol *this,
>  {
>  	struct efi_hii_packagelist *hii = package_list;
>  	struct efi_string_table *stbl;
> +	efi_status_t ret;
>  
>  	EFI_ENTRY("%p, \"%s\", %p, %u, %p, %p, %p", this, language,
>  		  package_list, string_id, string, string_size,
>  		  string_font_info);
>  
> -	if (!package_list || !efi_hii_packagelist_exists(package_list))
> -		return EFI_EXIT(EFI_NOT_FOUND);
> +	if (!language || !string_size || (*string_size && !string)) {
> +		ret = EFI_INVALID_PARAMETER;
> +		goto out;
> +	}
> +
> +	if (!package_list ||
> +	    !efi_hii_packagelist_exists(package_list) ||
> +	    !string id ||

Missing "_" .......^

> +	    string_id > hii->max_string_id) {
> +		ret = EFI_NOT_FOUND;
> +		goto out;
> +	}
>  
> +	ret = EFI_INVALID_LANGUAGE;
>  	list_for_each_entry(stbl, &hii->string_tables, link) {
>  		if (language_match((char *)language, stbl->language)) {
>  			efi_string_t str;
>  			size_t len;
>  
>  			if (stbl->nstrings < string_id)
> -				return EFI_EXIT(EFI_NOT_FOUND);
> +				goto out;
>  
>  			str = stbl->strings[string_id - 1].string;
>  			if (str) {
>  				len = u16_strsize(str);
>  				if (*string_size < len) {
>  					*string_size = len;
> -
> -					return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
> +					ret = EFI_BUFFER_TOO_SMALL;
> +					goto out;
>  				}
> -				memcpy(string, str, len);
>  				*string_size = len;
> +				if (string)
> +					memcpy(string, str, len);
> +				ret = EFI_SUCCESS;
> +				goto out;
>  			} else {
> -				return EFI_EXIT(EFI_NOT_FOUND);
> +				goto out;
>  			}
> -
> -			return EFI_EXIT(EFI_SUCCESS);
>  		}
>  	}
>  
> -	return EFI_EXIT(EFI_NOT_FOUND);
> +out:
> +	return EFI_EXIT(ret);
>  }
>  
>  static efi_status_t EFIAPI
> -- 
> 2.53.0
> 

-- 
Vincent.