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 24A5AC4332F for ; Wed, 9 Nov 2022 14:50:58 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 0D3D285010; Wed, 9 Nov 2022 15:50: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=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="GGviw/HT"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id ABA7B84FE8; Wed, 9 Nov 2022 15:50:54 +0100 (CET) Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) (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 9A96984FE8 for ; Wed, 9 Nov 2022 15:50:51 +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-wr1-x434.google.com with SMTP id z14so26122797wrn.7 for ; Wed, 09 Nov 2022 06:50:51 -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=ZO2qWfF8KyOKSdjPoP0pOp1zsI4O9wQBJWP5r4izF70=; b=GGviw/HTsqe9uWqAdeNy8Z7zaOioLU10/oV5I1JMzRo7ViwcbpSk1n79TnCr9MAsLC F+KQEU4qsfTj6mgJRVZdPGxDBT37huI7izKAWIJS38JnziU5aGd42odlgtw9oPI6iKEp r5N/KjlffzoQrChY7RBNgkN6hwGbjQblVkrZ8BBnuCjapixN337I7uh6BW7ymvg2Y1U/ AH1hxJma49URsHo41TaIBmhRK3WduQSeTSwHRrxTZGAwZdMwyyoxyqnBuphrxMEG4Jmw wQ2CgWYXfEYcXZC1Y4tS3aZVcuozd3YYik5VIouqGGP3TNvHIk7CKK9Ym2bWI4O9EdDP 89GA== 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=ZO2qWfF8KyOKSdjPoP0pOp1zsI4O9wQBJWP5r4izF70=; b=ZAA/+5e9NYCF+lLWdEQ5VcqSReRYSw8C/k3LjxjOTlBO2LcF5qCT3fOJ9EXHi2SnYk z9PyfRIjnkjDVggSkG4CM/NoWNvlhR6TgO62Q1lYC6wKW0oNSD92Yk17N9OgsSKL2B1q EJPP2f7r+ZI5eVTZ3wWDahQcbelWQLWhmUBOpc36dyAKaNXfVgTbzvfNehJASCMKBAcd R5PTrkGgGAqIeQ5ehB29m/nmQPq/6RWtkOiLMMeXKnHwScvtvItDgjUDikymPIVxwdSo ceckC80gMwhztr1maYQkJ/1P8ZCodiXawCb4yK46xgMUOJ/NdcDXyx0rNjirsWrA8Bn5 Rf/g== X-Gm-Message-State: ACrzQf1Q01kSCPQRzKY+WM77XcP0EdLgukG7bvclf/aCBuj6TqezPAMo 1B/ShU+F+ooTNjEarwUSbaABe8g8rkQTrXHI X-Google-Smtp-Source: AMsMyM6RCkXcDgPF+r5hbTLzbfQIfrrJ66/X69xg6Xhe2UTg6WzIyxSFgqdWST9iJye41YR3zqQo5g== X-Received: by 2002:a05:6000:c3:b0:236:a261:a2a5 with SMTP id q3-20020a05600000c300b00236a261a2a5mr39283818wrx.137.1668005451073; Wed, 09 Nov 2022 06:50:51 -0800 (PST) Received: from hera (ppp046103046165.access.hol.gr. [46.103.46.165]) by smtp.gmail.com with ESMTPSA id h4-20020a05600c350400b003c6f426467fsm2010871wmq.40.2022.11.09.06.50.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 09 Nov 2022 06:50:50 -0800 (PST) Date: Wed, 9 Nov 2022 16:50:48 +0200 From: Ilias Apalodimas To: Masahisa Kojima Cc: kojima.masahisa@socionext.com, u-boot@lists.denx.de, Heinrich Schuchardt , Simon Glass , Takahiro Akashi , Etienne Carriere Subject: Re: [PATCH v7 3/5] eficonfig: refactor change boot order implementation Message-ID: References: <20221109033728.5623-1-masahisa.kojima@linaro.org> <20221109033728.5623-4-masahisa.kojima@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221109033728.5623-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 On Wed, Nov 09, 2022 at 12:37:26PM +0900, Masahisa Kojima wrote: > All the eficonfig menus other than "Change Boot Order" > use 'eficonfig_entry' structure for each menu entry. > This commit refactors change boot order implementation > to use 'eficonfig_entry' structure same as other menus > to have consistent menu handling. > > This commit also simplifies the data->active handling when > KEY_SPACE is pressed, and sizeof() parameter. > > Signed-off-by: Masahisa Kojima > --- > Changes in v7: > - simplify the data->active handling > - update sizeof() parameter > - update commit message > > Changes in v5: > - remove direct access mode > > newly created in v4 > > cmd/eficonfig.c | 129 +++++++++++++++++++++++++----------------------- > 1 file changed, 67 insertions(+), 62 deletions(-) > > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c > index b392de7954..12babb76c2 100644 > --- a/cmd/eficonfig.c > +++ b/cmd/eficonfig.c > @@ -93,20 +93,14 @@ struct eficonfig_boot_selection_data { > }; > > /** > - * struct eficonfig_boot_order - structure to be used to update BootOrder variable > + * struct eficonfig_boot_order_data - structure to be used to update BootOrder variable > * > - * @num: index in the menu entry > - * @description: pointer to the description string > * @boot_index: boot option index > * @active: flag to include the boot option into BootOrder variable > - * @list: list structure > */ > -struct eficonfig_boot_order { > - u32 num; > - u16 *description; > +struct eficonfig_boot_order_data { > u32 boot_index; > bool active; > - struct list_head list; > }; > > /** > @@ -1802,7 +1796,7 @@ static void eficonfig_display_change_boot_order(struct efimenu *efi_menu) > { > bool reverse; > struct list_head *pos, *n; > - struct eficonfig_boot_order *entry; > + struct eficonfig_entry *entry; > > printf(ANSI_CLEAR_CONSOLE ANSI_CURSOR_POSITION > "\n ** Change Boot Order **\n" > @@ -1818,7 +1812,7 @@ static void eficonfig_display_change_boot_order(struct efimenu *efi_menu) > > /* draw boot option list */ > 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); > reverse = (entry->num == efi_menu->active); > > printf(ANSI_CURSOR_POSITION, entry->num + 4, 7); > @@ -1827,13 +1821,13 @@ static void eficonfig_display_change_boot_order(struct efimenu *efi_menu) > puts(ANSI_COLOR_REVERSE); > > if (entry->num < efi_menu->count - 2) { > - if (entry->active) > + if (((struct eficonfig_boot_order_data *)entry->data)->active) > printf("[*] "); > else > printf("[ ] "); > } > > - printf("%ls", entry->description); > + printf("%s", entry->title); > > if (reverse) > puts(ANSI_COLOR_RESET); > @@ -1850,9 +1844,8 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu) > { > int esc = 0; > struct list_head *pos, *n; > - struct eficonfig_boot_order *tmp; > enum bootmenu_key key = KEY_NONE; > - struct eficonfig_boot_order *entry; > + struct eficonfig_entry *entry, *tmp; > > while (1) { > bootmenu_loop(NULL, &key, &esc); > @@ -1861,11 +1854,11 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu) > case KEY_PLUS: > if (efi_menu->active > 0) { > 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->prev, struct eficonfig_boot_order, list); > + tmp = list_entry(pos->prev, struct eficonfig_entry, list); > entry->num--; > tmp->num++; > list_del(&tmp->list); > @@ -1879,11 +1872,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); > @@ -1909,9 +1902,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; > return EFI_NOT_READY; > } > } > @@ -1937,12 +1932,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); > @@ -1950,31 +1946,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(*data)); > + if (!data) { > + ret = EFI_OUT_OF_RESOURCES; > + goto out; > } > > - entry = calloc(1, sizeof(struct eficonfig_boot_order)); > - 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); > + goto out; > } > - entry->num = efi_menu->count++; > - entry->boot_index = boot_index; > - entry->active = active; > - list_add_tail(&entry->list, &efi_menu->list); > > +out: > free(load_option); > > - return EFI_SUCCESS; > + return ret; > } > > /** > @@ -1989,8 +1992,8 @@ static efi_status_t eficonfig_create_change_boot_order_entry(struct efimenu *efi > u16 *bootorder, efi_uintn_t num) > { > u32 i; > + char *title; > efi_status_t ret; > - struct eficonfig_boot_order *entry; > > /* list the load option in the order of BootOrder variable */ > for (i = 0; i < num; i++) { > @@ -2017,27 +2020,25 @@ static efi_status_t eficonfig_create_change_boot_order_entry(struct efimenu *efi > } > > /* add "Save" and "Quit" entries */ > - entry = calloc(1, sizeof(struct eficonfig_boot_order)); > - if (!entry) > + title = strdup("Save"); > + if (!title) { > + ret = EFI_OUT_OF_RESOURCES; > goto out; > + } > > - entry->num = efi_menu->count++; > - entry->description = u16_strdup(u"Save"); > - list_add_tail(&entry->list, &efi_menu->list); > - > - entry = calloc(1, sizeof(struct eficonfig_boot_order)); > - if (!entry) > + ret = eficonfig_append_menu_entry(efi_menu, title, NULL, NULL); > + if (ret != EFI_SUCCESS) > goto out; > > - entry->num = efi_menu->count++; > - entry->description = u16_strdup(u"Quit"); > - list_add_tail(&entry->list, &efi_menu->list); > + ret = eficonfig_append_quit_entry(efi_menu); > + if (ret != EFI_SUCCESS) > + goto out; > > efi_menu->active = 0; > > return EFI_SUCCESS; > out: > - return EFI_OUT_OF_RESOURCES; > + return ret; > } > > /** > @@ -2053,7 +2054,7 @@ static efi_status_t eficonfig_process_change_boot_order(void *data) > efi_status_t ret; > efi_uintn_t num, size; > struct list_head *pos, *n; > - struct eficonfig_boot_order *entry; > + struct eficonfig_entry *entry; > struct efimenu *efi_menu; > > efi_menu = calloc(1, sizeof(struct efimenu)); > @@ -2084,9 +2085,16 @@ static efi_status_t eficonfig_process_change_boot_order(void *data) > /* create new BootOrder */ > count = 0; > list_for_each_safe(pos, n, &efi_menu->list) { > - entry = list_entry(pos, struct eficonfig_boot_order, list); > - if (entry->active) > - new_bootorder[count++] = entry->boot_index; > + struct eficonfig_boot_order_data *data; > + > + entry = list_entry(pos, struct eficonfig_entry, list); > + /* exit the loop when iteration reaches "Save" */ > + if (!strncmp(entry->title, "Save", strlen("Save"))) > + break; > + > + data = entry->data; > + if (data->active) > + new_bootorder[count++] = data->boot_index; > } > > size = count * sizeof(u16); > @@ -2105,15 +2113,12 @@ static efi_status_t eficonfig_process_change_boot_order(void *data) > } > } > out: > + free(bootorder); > list_for_each_safe(pos, n, &efi_menu->list) { > - entry = list_entry(pos, struct eficonfig_boot_order, list); > - list_del(&entry->list); > - free(entry->description); > - free(entry); > + entry = list_entry(pos, struct eficonfig_entry, list); > + free(entry->data); > } > - > - free(bootorder); > - free(efi_menu); > + eficonfig_destroy(efi_menu); > > /* to stay the parent menu */ > ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret; > -- > 2.17.1 > Acked-by: Ilias Apalodimas