From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 2723CC4332F for ; Tue, 20 Dec 2022 06:44:50 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 9BAD784881; Tue, 20 Dec 2022 07:44:47 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="qfYoHUC9"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id F267583D10; Tue, 20 Dec 2022 07:44:45 +0100 (CET) Received: from mail-ed1-x535.google.com (mail-ed1-x535.google.com [IPv6:2a00:1450:4864:20::535]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 9929683673 for ; Tue, 20 Dec 2022 07:44:43 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ilias.apalodimas@linaro.org Received: by mail-ed1-x535.google.com with SMTP id m21so5651008edc.3 for ; Mon, 19 Dec 2022 22:44:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=nrLeiO56ucAvemkeOKEVAs1ZkCo1OtAlBP/a9815axQ=; b=qfYoHUC9FHyv3GcKqFaV/pfrG7fmPvWM5R2lSXpX4PUOymS0Osb/r+GmhwI9xqqKD3 2yBiICI3MMUrFVvx/g3cFySfN1Fk8tLJPpyBY0A9patGL7EkL1CsYhrjWstzBlkjixwR 4BrEKffe0jauW529UXgzNLJxtXuy1GKsAF3hH6wo2B6xBxVXlAMHCZ0l9W7RMYIDBGK0 jSkOpE86XG7A0yrMudIpUmSXTnU3vSzx+02ahfd6TR7iBkDTr9O/Zpvzyk/w082LPSr4 +xRijeUxvGjnZZb2rAkop97vW64eNdCSN1J4737ty6lfbAyzusiq1I0tIShgmjpDhEcP 1pyQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=nrLeiO56ucAvemkeOKEVAs1ZkCo1OtAlBP/a9815axQ=; b=uKeVfqAKG5tFUobR8iQ6JBWkvI0mhVkD25WTtXGWN8UPXn5G1Z3fd5roRj33MGKQIP ykKsra/CrMixJpcLqbXt5XeQLDyJZ77YnJFnm5fXAouSCbLtyOziL+wfNiABscUWDH/T VLe+cCnyiQnsHusZv9QHY+eU7MhuiGxutRziVri2BfqnNmxJGK2SdW0UuilRrFXFKYNJ LNsZxt7G07qSpaEk5YfxLhyYvUZTioYcTdtzP9Jjv57NE227edTyvCz8kvWKTogOrLpy 5l3D0j9qYxLrkbh0wGkA8SPY5ZYF40BsPldZ2A3w6G5Wn4SjDP1HLSo1pWG2es8Pm3TR eMFw== X-Gm-Message-State: ANoB5pnfbwa4FDhHfOzxU07pecqmUXu+LeYIX+2bhR1QIZN5BpC42Q64 NTedsFskCocPBi66bYPubyVEJw== X-Google-Smtp-Source: AA0mqf6eQfM7S/miu9kUnPsoPZdefTeBEylgfmELQ3ZihHYyvxIze2qSILnxa9+KGBEuf7kScMbpPA== X-Received: by 2002:a05:6402:294b:b0:461:ca30:653 with SMTP id ed11-20020a056402294b00b00461ca300653mr48992859edb.31.1671518683162; Mon, 19 Dec 2022 22:44:43 -0800 (PST) Received: from hera (ppp079167090036.access.hol.gr. [79.167.90.36]) by smtp.gmail.com with ESMTPSA id bt16-20020a0564020a5000b0046ee136fa3bsm5183879edb.69.2022.12.19.22.44.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Dec 2022 22:44:42 -0800 (PST) Date: Tue, 20 Dec 2022 08:44:40 +0200 From: Ilias Apalodimas To: Masahisa Kojima Cc: u-boot@lists.denx.de, Heinrich Schuchardt Subject: Re: [PATCH v2 1/2] eficonfig: carve out efi_get_next_variable_name_int calls Message-ID: References: <20221219023314.23959-1-masahisa.kojima@linaro.org> <20221219023314.23959-2-masahisa.kojima@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221219023314.23959-2-masahisa.kojima@linaro.org> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On Mon, Dec 19, 2022 at 11:33:12AM +0900, Masahisa Kojima wrote: > To retrieve the EFI variable name by efi_get_next_variable_name_int(), > the sequence of alloc -> efi_get_next_variable_name_int -> > realloc -> efi_get_next_variable_name_int is required. > In current code, this sequence repeatedly appears in > the several functions. It should be curved out a common function. > > This commit also fixes the missing free() of var_name16 > in eficonfig_delete_invalid_boot_option(). > > Signed-off-by: Masahisa Kojima > --- > Changes in v2: > - fix typo in the commit message > - rename efi_get_variable_name to efi_next_variable_name > > cmd/eficonfig.c | 62 +++++++++---------------------------- > include/efi_loader.h | 2 ++ > lib/efi_loader/efi_helper.c | 34 ++++++++++++++++++++ > 3 files changed, 51 insertions(+), 47 deletions(-) > > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c > index 394ae67cce..0b07dfc958 100644 > --- a/cmd/eficonfig.c > +++ b/cmd/eficonfig.c > @@ -1683,7 +1683,7 @@ static efi_status_t eficonfig_show_boot_selection(unsigned int *selected) > u32 i; > u16 *bootorder; > efi_status_t ret; > - u16 *var_name16 = NULL, *p; > + u16 *var_name16 = NULL; > efi_uintn_t num, size, buf_size; > struct efimenu *efi_menu; > struct list_head *pos, *n; > @@ -1718,24 +1718,12 @@ static efi_status_t eficonfig_show_boot_selection(unsigned int *selected) > int index; > efi_guid_t guid; > > - size = buf_size; > - ret = efi_get_next_variable_name_int(&size, var_name16, &guid); > + ret = efi_next_variable_name(&buf_size, &var_name16, &guid); > if (ret == EFI_NOT_FOUND) > break; > - if (ret == EFI_BUFFER_TOO_SMALL) { > - buf_size = size; > - p = realloc(var_name16, buf_size); > - if (!p) { > - free(var_name16); > - return EFI_OUT_OF_RESOURCES; > - } > - var_name16 = p; > - ret = efi_get_next_variable_name_int(&size, var_name16, &guid); > - } > - if (ret != EFI_SUCCESS) { > - free(var_name16); > - return ret; > - } > + if (ret != EFI_SUCCESS) > + goto out; > + > if (efi_varname_is_load_option(var_name16, &index)) { > /* If the index is included in the BootOrder, skip it */ > if (search_bootorder(bootorder, num, index, NULL)) > @@ -2026,7 +2014,7 @@ static efi_status_t eficonfig_create_change_boot_order_entry(struct efimenu *efi > u32 i; > char *title; > efi_status_t ret; > - u16 *var_name16 = NULL, *p; > + u16 *var_name16 = NULL; > efi_uintn_t size, buf_size; > > /* list the load option in the order of BootOrder variable */ > @@ -2054,19 +2042,9 @@ static efi_status_t eficonfig_create_change_boot_order_entry(struct efimenu *efi > break; > > size = buf_size; > - ret = efi_get_next_variable_name_int(&size, var_name16, &guid); > + ret = efi_next_variable_name(&buf_size, &var_name16, &guid); > if (ret == EFI_NOT_FOUND) > break; > - if (ret == EFI_BUFFER_TOO_SMALL) { > - buf_size = size; > - p = realloc(var_name16, buf_size); > - if (!p) { > - ret = EFI_OUT_OF_RESOURCES; > - goto out; > - } > - var_name16 = p; > - ret = efi_get_next_variable_name_int(&size, var_name16, &guid); > - } > if (ret != EFI_SUCCESS) > goto out; > > @@ -2336,10 +2314,10 @@ efi_status_t eficonfig_delete_invalid_boot_option(struct eficonfig_media_boot_op > efi_uintn_t size; > void *load_option; > struct efi_load_option lo; > - u16 *var_name16 = NULL, *p; > + u16 *var_name16 = NULL; > u16 varname[] = u"Boot####"; > efi_status_t ret = EFI_SUCCESS; > - efi_uintn_t varname_size, buf_size; > + efi_uintn_t buf_size; > > buf_size = 128; > var_name16 = malloc(buf_size); > @@ -2352,24 +2330,12 @@ efi_status_t eficonfig_delete_invalid_boot_option(struct eficonfig_media_boot_op > efi_guid_t guid; > efi_uintn_t tmp; > > - varname_size = buf_size; > - ret = efi_get_next_variable_name_int(&varname_size, var_name16, &guid); > + ret = efi_next_variable_name(&buf_size, &var_name16, &guid); > if (ret == EFI_NOT_FOUND) > break; > - if (ret == EFI_BUFFER_TOO_SMALL) { > - buf_size = varname_size; > - p = realloc(var_name16, buf_size); > - if (!p) { > - free(var_name16); > - return EFI_OUT_OF_RESOURCES; > - } > - var_name16 = p; > - ret = efi_get_next_variable_name_int(&varname_size, var_name16, &guid); > - } > - if (ret != EFI_SUCCESS) { > - free(var_name16); > - return ret; > - } > + if (ret != EFI_SUCCESS) > + goto out; > + > if (!efi_varname_is_load_option(var_name16, &index)) > continue; > > @@ -2407,6 +2373,8 @@ next: > } > > out: > + free(var_name16); > + > return ret; > } > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index 0899e293e5..699176872d 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -708,6 +708,8 @@ int algo_to_len(const char *algo); > int efi_link_dev(efi_handle_t handle, struct udevice *dev); > int efi_unlink_dev(efi_handle_t handle); > bool efi_varname_is_load_option(u16 *var_name16, int *index); > +efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf, > + efi_guid_t *guid); > > /** > * efi_size_in_pages() - convert size in bytes to size in pages > diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c > index 788cb9faec..1f4ab2b419 100644 > --- a/lib/efi_loader/efi_helper.c > +++ b/lib/efi_loader/efi_helper.c > @@ -223,3 +223,37 @@ bool efi_varname_is_load_option(u16 *var_name16, int *index) > > return false; > } > + > +/** > + * efi_next_variable_name() - get next variable name > + * > + * This function is a wrapper of efi_get_next_variable_name_int(). > + * If efi_get_next_variable_name_int() returns EFI_BUFFER_TOO_SMALL, > + * @size and @buf are updated by new buffer size and realloced buffer. > + * > + * @size: pointer to the buffer size > + * @buf: pointer to the buffer > + * @guid: pointer to the guid > + * Return: status code > + */ > +efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf, efi_guid_t *guid) > +{ > + u16 *p; > + efi_status_t ret; > + efi_uintn_t buf_size = *size; > + > + ret = efi_get_next_variable_name_int(&buf_size, *buf, guid); > + if (ret == EFI_NOT_FOUND) > + return ret; > + if (ret == EFI_BUFFER_TOO_SMALL) { > + p = realloc(*buf, buf_size); > + if (!p) > + return EFI_OUT_OF_RESOURCES; > + > + *buf = p; > + *size = buf_size; > + ret = efi_get_next_variable_name_int(&buf_size, *buf, guid); > + } > + > + return ret; > +} > -- > 2.17.1 > Reviewed-by: Ilias Apalodimas