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 94DABC4332F for ; Fri, 4 Nov 2022 22:09:03 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 2FD2683F5C; Fri, 4 Nov 2022 23:09:00 +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="F8hqDiW4"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 6CF2F83F26; Fri, 4 Nov 2022 23:08:58 +0100 (CET) Received: from mail-ed1-x536.google.com (mail-ed1-x536.google.com [IPv6:2a00:1450:4864:20::536]) (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 01D2A83F26 for ; Fri, 4 Nov 2022 23:08:56 +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-x536.google.com with SMTP id l11so9572628edb.4 for ; Fri, 04 Nov 2022 15:08:55 -0700 (PDT) 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=jT+qYv9xt3M6PYiJgTyQy5SGAZHbH+LjXlhTCiw15n8=; b=F8hqDiW4DP0RD5QB7YQe5okUO3V+sXr2ms18NuwlcCOH/vBWfLdGB/BNR67C1a03IO 3hm5kM9DuBV45VFg2D6Zjb0qC6hFDy6kr2w43/+DeDtuHdS1qqV/5RYsFPgaXgUwrJGA Wcj1ekc12D4H7YBCBHsNh+026Y3vxtIyGx0vuwlvWiyf0TnUcZvJa1cKPg+nhdHCABHu xD3Q0sABduquTudqIwjFUU4R6mSfOSeDAsBxVY1GS2upO+fnK7KEOFAb5X8kBrjUdiEX XxEOZgWqMBLhNt323L5LTiyfi/qYYRRdKJJ5+HAU7uLl3qBGKPBdgPHMeNBPwTBSnFwI odSg== 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=jT+qYv9xt3M6PYiJgTyQy5SGAZHbH+LjXlhTCiw15n8=; b=3Vds4NxzUxTORwCkV0ZlFF826uu2VniEKlL7fTmHxXXZYJllT/7z1M+huN4lztZXrp 5iizfyfQ22HxEHSBRMG79iH1ZEtb+pqFsKCsm33EKImociwj54gOqDwnbaDO5zaMME24 LJT7eCwnwf6pWf4yjgu/NcCj0e5JtWkv5Nrm8hobeKmHBhgDYHNHMg/agaWQLnwk/1VG WH5Lmi1jUfMpUsDdqt3EEi5yhsuRcrm5qwdy9Ug0EWSQ7/Fu4pp9+4U7p0owQihVCKkR pYx37L5VQTM4Rn/JvRfn0xGKU3BeUTTKOfW5K2cwOm5v3z0vQhP0ZHhMmczu9yqOzk7e QOKA== X-Gm-Message-State: ACrzQf2yCV8dyzj0wWU+Zy1DoWF9aHD2PFtQSKNGyKOmsBoV30ixZvX1 QPbOsEdcsZLDzXjcHc3I/f2y1g== X-Google-Smtp-Source: AMsMyM6veUvgUv9vV7s/driEOjUnS95/cKB5SF8NJvI2Nq7LlFV65LSVkNiaUxd9i3hpO7ruNCVzYQ== X-Received: by 2002:a05:6402:50c:b0:461:bc01:1828 with SMTP id m12-20020a056402050c00b00461bc011828mr38525900edv.64.1667599735590; Fri, 04 Nov 2022 15:08:55 -0700 (PDT) Received: from hera (ppp046103046165.access.hol.gr. [46.103.46.165]) by smtp.gmail.com with ESMTPSA id x19-20020a170906711300b0078116c361d9sm102728ejj.10.2022.11.04.15.08.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 04 Nov 2022 15:08:55 -0700 (PDT) Date: Sat, 5 Nov 2022 00:08:52 +0200 From: Ilias Apalodimas To: Masahisa Kojima Cc: u-boot@lists.denx.de, Heinrich Schuchardt , Simon Glass , Takahiro Akashi , Etienne Carriere Subject: Re: [PATCH v6 3/5] eficonfig: refactor change boot order implementation Message-ID: References: <20221026104345.28714-1-masahisa.kojima@linaro.org> <20221026104345.28714-4-masahisa.kojima@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221026104345.28714-4-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 Hi Kojima-san On Wed, Oct 26, 2022 at 07:43:43PM +0900, Masahisa Kojima wrote: > This commit refactors change boot order implementation > to use 'eficonfig_entry' structure. Please add an explanation on why we are doing this, instead of what the patch is doing. I am assuming it cleans up some code and allows us to reuse eficonfig_entry since ->data is now pointing to an eficonfig_boot_order_data struct? > > Signed-off-by: Masahisa Kojima > --- > No update since v5 > > Changes in v5: > - remove direct access mode > > newly created in v4 > > cmd/eficonfig.c | 129 +++++++++++++++++++++++++----------------------- > 1 file changed, 67 insertions(+), 62 deletions(-) > > list_del(&tmp->list); [...] > @@ -1891,11 +1884,11 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu) > case KEY_MINUS: > if (efi_menu->active < efi_menu->count - 3) { > list_for_each_safe(pos, n, &efi_menu->list) { > - entry = list_entry(pos, struct eficonfig_boot_order, list); > + entry = list_entry(pos, struct eficonfig_entry, list); > if (entry->num == efi_menu->active) > break; > } > - tmp = list_entry(pos->next, struct eficonfig_boot_order, list); > + tmp = list_entry(pos->next, struct eficonfig_entry, list); > entry->num++; > tmp->num--; > list_del(&entry->list); > @@ -1921,9 +1914,11 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu) > case KEY_SPACE: > if (efi_menu->active < efi_menu->count - 2) { > list_for_each_safe(pos, n, &efi_menu->list) { > - entry = list_entry(pos, struct eficonfig_boot_order, list); > + entry = list_entry(pos, struct eficonfig_entry, list); > if (entry->num == efi_menu->active) { > - entry->active = entry->active ? false : true; > + struct eficonfig_boot_order_data *data = entry->data; > + > + data->active = data->active ? false : true; data->active = !!data->active seems a bit better here imho > return EFI_NOT_READY; > } > } > @@ -1949,12 +1944,13 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu) > static efi_status_t eficonfig_add_change_boot_order_entry(struct efimenu *efi_menu, > u32 boot_index, bool active) > { > + char *title, *p; > efi_status_t ret; > efi_uintn_t size; > void *load_option; > struct efi_load_option lo; > u16 varname[] = u"Boot####"; > - struct eficonfig_boot_order *entry; > + struct eficonfig_boot_order_data *data; > > efi_create_indexed_name(varname, sizeof(varname), "Boot", boot_index); > load_option = efi_get_var(varname, &efi_global_variable_guid, &size); > @@ -1962,31 +1958,38 @@ static efi_status_t eficonfig_add_change_boot_order_entry(struct efimenu *efi_me > return EFI_SUCCESS; > > ret = efi_deserialize_load_option(&lo, load_option, &size); > - if (ret != EFI_SUCCESS) { > - free(load_option); > - return ret; > + if (ret != EFI_SUCCESS) > + goto out; > + > + data = calloc(1, sizeof(struct eficonfig_boot_order_data)); sizeof(*data) > + if (!data) { > + ret = EFI_OUT_OF_RESOURCES; > + goto out; > } > > - entry = calloc(1, sizeof(struct eficonfig_boot_order)); sizeof(*entry) > - if (!entry) { > - free(load_option); > - return EFI_OUT_OF_RESOURCES; > + title = calloc(1, utf16_utf8_strlen(lo.label) + 1); > + if (!title) { > + free(data); > + ret = EFI_OUT_OF_RESOURCES; > + goto out; > } > + p = title; > + utf16_utf8_strcpy(&p, lo.label); > > - entry->description = u16_strdup(lo.label); > - if (!entry->description) { > - free(load_option); > - free(entry); > - return EFI_OUT_OF_RESOURCES; > + data->boot_index = boot_index; > + data->active = active; > + > + ret = eficonfig_append_menu_entry(efi_menu, title, NULL, data); > + if (ret != EFI_SUCCESS) { > + free(data); > + free(title); Thanks /Ilias