[{"id":1769862,"web_url":"http://patchwork.ozlabs.org/comment/1769862/","msgid":"<3949db75-20a0-33d2-e879-3a42ea0d85ce@gmx.de>","list_archive_url":null,"date":"2017-09-18T04:33:25","subject":"Re: [U-Boot] [PATCH 03/16] efi: Add error checking for\n\tefi_init_obj_list()","submitter":{"id":61270,"url":"http://patchwork.ozlabs.org/api/people/61270/","name":"Heinrich Schuchardt","email":"xypron.glpk@gmx.de"},"content":"On 09/18/2017 12:59 AM, Simon Glass wrote:\n> This function calls a function which can fail. Print a message in this\n> case and abort the boot, rather than silently continuing to boot, which\n> will certainly fail.\n> \n> Signed-off-by: Simon Glass \n> ---\n> \n> cmd/bootefi.c | 30 ++++++++++++++++++++++++------\n> 1 file changed, 24 insertions(+), 6 deletions(-)\n> \n> diff --git a/cmd/bootefi.c b/cmd/bootefi.c\n> index 2c9d31c5eb..9aa588eb1b 100644\n> --- a/cmd/bootefi.c\n> +++ b/cmd/bootefi.c\n> @@ -106,11 +106,17 @@ static struct efi_object bootefi_device_obj = {\n> \t},\n> };\n> \n> -/* Initialize and populate EFI object list */\n> -static void efi_init_obj_list(void)\n> +/**\n> + * efi_init_obj_list() - Initialize and populate EFI object list\n> + *\n> + * @return 0 if OK, -ve on error (in which case it prints a message)\n> + */\n> +static int efi_init_obj_list(void)\n\nUse efi_status_t as return type.\n\n> {\n> +\tint ret;\n> +\n> \tif (efi_obj_list_initalized)\n> -\t\treturn;\n> +\t\treturn 0;\n> \tefi_obj_list_initalized = 1;\n> \n> \tlist_add_tail(&loaded_image_info_obj.link, &efi_obj_list);\n> @@ -132,12 +138,19 @@ static void efi_init_obj_list(void)\n> \t\tloaded_image_info.device_handle = bootefi_device_path;\n> #endif\n> #ifdef CONFIG_GENERATE_SMBIOS_TABLE\n> -\tefi_smbios_register();\n> +\tret = efi_smbios_register();\n> +\tif (ret)\n> +\t\tgoto error;\n> #endif\n> \n> \t/* Initialize EFI runtime services */\n> \tefi_reset_system_init();\n> \tefi_get_time_init();\n> +\n> +\treturn 0;\n> +error:\n> +\tprintf(\"Error: Cannot set up EFI object list (err=%d)\\n\", ret);\n> +\treturn ret;\n> }\n> \n> static void *copy_fdt(void *fdt)\n> @@ -219,6 +232,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)\n> \tulong fdt_pages, fdt_size, fdt_start, fdt_end;\n> \tconst efi_guid_t fdt_guid = EFI_FDT_GUID;\n> \tbootm_headers_t img = { 0 };\n> +\tint ret;\n> \n> \t/*\n> \t * gd lives in a fixed register which may get clobbered while we execute\n> @@ -258,7 +272,9 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)\n> \t\treturn -ENOENT;\n> \n> \t/* Initialize and populate EFI object list */\n> -\tefi_init_obj_list();\n> +\tret = efi_init_obj_list();\n> +\tif (ret)\n> +\t\treturn ret;\n> \n> \t/* Call our payload! */\n> \tdebug(\"%s:%d Jumping to 0x%lx\\n\", __func__, __LINE__, (long)entry);\n> @@ -312,7 +328,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])\n> \t\t */\n> \t\tefi_save_gd();\n> \t\t/* Initialize and populate EFI object list */\n> -\t\tefi_init_obj_list();\n> +\t\tif (efi_init_obj_list())\n> +\t\t\treturn CMD_RET_FAILURE;\n> +\n\nWe are duplicating code here.\n\nefi_save_gd and efi_init_obj_list should have been moved to the start of\ndo_bootefi_exec in my patch\nefi_selftest: provide unit test for event services\n\n> \t\tloaded_image_info.device_handle = bootefi_device_path;\n> \t\tloaded_image_info.file_path = bootefi_image_path;\n> \t\treturn efi_selftest(&loaded_image_info, &systab);\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3xwY6y3P2Bz9s7G\n\tfor ;\n\tMon, 18 Sep 2017 14:34:01 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid D26C3C21EE8; Mon, 18 Sep 2017 04:33:47 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 1CC22C21D57;\n\tMon, 18 Sep 2017 04:33:45 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid A3C6AC21D19; Mon, 18 Sep 2017 04:33:43 +0000 (UTC)","from mout.gmx.net (mout.gmx.net [212.227.17.21])\n\tby lists.denx.de (Postfix) with ESMTPS id 50909C21D19\n\tfor ; Mon, 18 Sep 2017 04:33:43 +0000 (UTC)","from [192.168.123.58] ([84.118.154.110]) by mail.gmx.com (mrgmx101\n\t[212.227.17.168]) with ESMTPSA (Nemesis) id\n\t0MfRnb-1e3v2R0t23-00P2kj; Mon, 18 Sep 2017 06:33:28 +0200"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.7 required=5.0 tests=FREEMAIL_FROM,\n\tRCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3,\n\tRCVD_IN_MSPIKE_WL autolearn=unavailable\n\tautolearn_force=no version=3.4.0","To":"Simon Glass ,\n\tU-Boot Mailing List ","References":"<20170917225927.117917-1-sjg@chromium.org>\n\t<20170917225927.117917-4-sjg@chromium.org>","From":"Heinrich Schuchardt ","Message-ID":"<3949db75-20a0-33d2-e879-3a42ea0d85ce@gmx.de>","Date":"Mon, 18 Sep 2017 06:33:25 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170917225927.117917-4-sjg@chromium.org>","Content-Language":"en-US","X-Provags-ID":"V03:K0:zlKJj6T4mJGtYHFhvQzi/SpPNwA6xXzBXphqchrVlvi8Bm6ePMt\n\tdhX7OBIlZQczekh4HmHfG3PJneoD1FfBoCkw6RL7YAc0K71FaCzm+7fm2TPP4kRVWv0Tq5g\n\tJvDDxtWcM6oJD9SVbRlbqLR0LKzAIpJ49NhFIOfAYAtbLphNEoAvI6jJ5BnPdIH+XUTCQEv\n\t8zjqGFZQrykERMQjAazUA==","X-UI-Out-Filterresults":"notjunk:1; V01:K0:azfejP1N9L0=:5Mj0Rx9YSpwiCNXlWGDPjG\n\tqHfi97mJ2QYdLQSuLFARz2E+45RjzZW6Vefc8mbbJfZ6COjP2fw7I4NMSJTuC3qKowYs0BZuk\n\tx9qcUZfMBeI6/8KojXwWn7oJQPpsHeh+KqiKxiYzxlU8XDrHoDzf4ku+iAV0PTZ4J4WCYjqVT\n\tdP71jlVd1V7hqznw0YIjB/Dy5zKWS7Pe//M4SOEJVVErBU/iExSggCKIHg6l/+SltU5LzpYZe\n\tJcdNS33lWYWFGXSWm8L+TrCAj1850asoS6IDFa420Uo36N1RpWj0FhreBaq2QJLj3N3jnY9w4\n\txU7lRm/TZ9ttpwR74nJM5e0gcEqZ9NWolxJSzS9ugYWciADsmiteMtwU7+XvRW6ZZ0l9md7qW\n\t12eq2l0cjUeneBT13R8hWwRUVPUcYCSqbOzOoaaraqmJ4zhH2STNrLj1TU56UjNYMHaDOpW2O\n\t7b1AWr97JUAjl+jZe5t3OWi8gSsK+B+ulVNgkmIzhAp/d7MK43JxoX0q4HGVowzUBOmXXiQlb\n\tC+Bih/kBZHJ9fwpSADRHN/KiTbo6lfCB8+uB7x53LqsTt4HdbQRyApe0NDCTnd999b3I9fdKq\n\tagnsBcUjQfZeYnsgHB2BO0Eu52+FDPkG1BAz9p9KUJ6kJqU4FF/d4cTpGiTF8M+N+ITTq/VhH\n\tXVNaZdR2KMNYFkcgvmxldmFrKAAnusdpYJqU6qaa7RnVJ8sjouvu3n0e73YaDjBvIjkasRGGY\n\tPLZ00lMRhbWLPYbff736197ojonGfaXOCk2LELUEam1OJwTXxM72PQBs10SrtIYES8hmQzRAs\n\tRgEgjA22TZr7X6eSPwa6Uobl0EWtAYni63v2+iR4RpX5Z5Id04=","Cc":"Heinrich Schuchardt ,\n\tAndy Shevchenko ","Subject":"Re: [U-Boot] [PATCH 03/16] efi: Add error checking for\n\tefi_init_obj_list()","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}},{"id":1815659,"web_url":"http://patchwork.ozlabs.org/comment/1815659/","msgid":"","list_archive_url":null,"date":"2017-12-04T22:45:03","subject":"Re: [U-Boot] [PATCH 03/16] efi: Add error checking for\n\tefi_init_obj_list()","submitter":{"id":6170,"url":"http://patchwork.ozlabs.org/api/people/6170/","name":"Simon Glass","email":"sjg@chromium.org"},"content":"Hi Heinrich,\n\nOn 17 September 2017 at 22:33, Heinrich Schuchardt wrote:\n>\n> On 09/18/2017 12:59 AM, Simon Glass wrote:\n> > This function calls a function which can fail. Print a message in this\n> > case and abort the boot, rather than silently continuing to boot, which\n> > will certainly fail.\n> >\n> > Signed-off-by: Simon Glass \n> > ---\n> >\n> > cmd/bootefi.c | 30 ++++++++++++++++++++++++------\n> > 1 file changed, 24 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/cmd/bootefi.c b/cmd/bootefi.c\n> > index 2c9d31c5eb..9aa588eb1b 100644\n> > --- a/cmd/bootefi.c\n> > +++ b/cmd/bootefi.c\n> > @@ -106,11 +106,17 @@ static struct efi_object bootefi_device_obj = {\n> > },\n> > };\n> >\n> > -/* Initialize and populate EFI object list */\n> > -static void efi_init_obj_list(void)\n> > +/**\n> > + * efi_init_obj_list() - Initialize and populate EFI object list\n> > + *\n> > + * @return 0 if OK, -ve on error (in which case it prints a message)\n> > + */\n> > +static int efi_init_obj_list(void)\n>\n> Use efi_status_t as return type.\n>\n> > {\n> > + int ret;\n> > +\n> > if (efi_obj_list_initalized)\n> > - return;\n> > + return 0;\n> > efi_obj_list_initalized = 1;\n> >\n> > list_add_tail(&loaded_image_info_obj.link, &efi_obj_list);\n> > @@ -132,12 +138,19 @@ static void efi_init_obj_list(void)\n> > loaded_image_info.device_handle = bootefi_device_path;\n> > #endif\n> > #ifdef CONFIG_GENERATE_SMBIOS_TABLE\n> > - efi_smbios_register();\n> > + ret = efi_smbios_register();\n> > + if (ret)\n> > + goto error;\n> > #endif\n> >\n> > /* Initialize EFI runtime services */\n> > efi_reset_system_init();\n> > efi_get_time_init();\n> > +\n> > + return 0;\n> > +error:\n> > + printf(\"Error: Cannot set up EFI object list (err=%d)\\n\", ret);\n> > + return ret;\n> > }\n> >\n> > static void *copy_fdt(void *fdt)\n> > @@ -219,6 +232,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)\n> > ulong fdt_pages, fdt_size, fdt_start, fdt_end;\n> > const efi_guid_t fdt_guid = EFI_FDT_GUID;\n> > bootm_headers_t img = { 0 };\n> > + int ret;\n> >\n> > /*\n> > * gd lives in a fixed register which may get clobbered while we execute\n> > @@ -258,7 +272,9 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)\n> > return -ENOENT;\n> >\n> > /* Initialize and populate EFI object list */\n> > - efi_init_obj_list();\n> > + ret = efi_init_obj_list();\n> > + if (ret)\n> > + return ret;\n> >\n> > /* Call our payload! */\n> > debug(\"%s:%d Jumping to 0x%lx\\n\", __func__, __LINE__, (long)entry);\n> > @@ -312,7 +328,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])\n> > */\n> > efi_save_gd();\n> > /* Initialize and populate EFI object list */\n> > - efi_init_obj_list();\n> > + if (efi_init_obj_list())\n> > + return CMD_RET_FAILURE;\n> > +\n>\n> We are duplicating code here.\n>\n> efi_save_gd and efi_init_obj_list should have been moved to the start of\n> do_bootefi_exec in my patch\n> efi_selftest: provide unit test for event services\n>\n\nYes I see that but I wonder if we should wait until your series is\napplied before figuring this out?\n\n>\n> > loaded_image_info.device_handle = bootefi_device_path;\n> > loaded_image_info.file_path = bootefi_image_path;\n> > return efi_selftest(&loaded_image_info, &systab);\n> >\n>\n\nRegards,\nSimon","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"UIE/XVLV\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"Ku/SCfrL\"; dkim-atps=neutral"],"Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3yrKhw22bcz9s74\n\tfor ;\n\tTue, 5 Dec 2017 09:45:36 +1100 (AEDT)","by lists.denx.de (Postfix, from userid 105)\n\tid D3C0EC2204C; Mon, 4 Dec 2017 22:45:30 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id C91ECC21D79;\n\tMon, 4 Dec 2017 22:45:27 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 60389C21DAA; Mon, 4 Dec 2017 22:45:26 +0000 (UTC)","from mail-qt0-f196.google.com (mail-qt0-f196.google.com\n\t[209.85.216.196])\n\tby lists.denx.de (Postfix) with ESMTPS id E0CAAC21D79\n\tfor ; Mon, 4 Dec 2017 22:45:25 +0000 (UTC)","by mail-qt0-f196.google.com with SMTP id w10so24865342qtb.10\n\tfor ; Mon, 04 Dec 2017 14:45:25 -0800 (PST)","by 10.140.100.167 with HTTP; Mon, 4 Dec 2017 14:45:03 -0800 (PST)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.0 required=5.0 tests=RCVD_IN_MSPIKE_H3,\n\tRCVD_IN_MSPIKE_WL,\n\tT_DKIM_INVALID autolearn=unavailable autolearn_force=no\n\tversion=3.4.0","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;\n\ts=20161025; \n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=eDcZNVHYV1Q5txDfQWMs0sLmVUofWIEWiVOwIao51RY=;\n\tb=UIE/XVLVhxAgIHziiAtTbCB8yVzrxlD10nA9RJNFLMuWGXp/WRi0NMfBSzdYmCF9gH\n\t+i1ASq+zE5ofclKMMa3au9QE/3y5+E4a1/g0EdFdZATJnV6YX3mpw0FAgmkAOPPAqcM/\n\t+2xZ6r2eThSsaZDbJM+AYnZsQu6nlHQbfswQwb2WRCWL6IrrswpMv5Tz1jzIdgKA9TU+\n\tGFloGmNE2Ay9sbklEGolPu7nuGzWCbO8LOeOqSxlTK0W2dmBBNghPLjmzMFBeSipIfJ4\n\tsr1QM1QJPkvQW/oSmtHj76jgztDYjEZQJxMSezeZf4v3GoS3G2YuobkZ54Yu218P3Rl1\n\tFByA==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=eDcZNVHYV1Q5txDfQWMs0sLmVUofWIEWiVOwIao51RY=;\n\tb=Ku/SCfrLTbyUFYduU4OwrfAGvdIT32TxQBNkWIx5katf0sgXNHyd7w0ZNuIr7sUYYJ\n\tWEu3nH+1zlzEJgTNLeXKosDOcC7Nzpjgnq95e9hbZmWISDmMqg5ZWDYph3bm3Bx3xKvY\n\tPOixthPDWF1P+lpObMsMDaIPzbfuJG1Q+A1hQ="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:sender:in-reply-to:references:from\n\t:date:message-id:subject:to:cc;\n\tbh=eDcZNVHYV1Q5txDfQWMs0sLmVUofWIEWiVOwIao51RY=;\n\tb=hNEdq3nYOlAk98yHAjKu5d7ZO6OH2gRqz+saluEmM15eg2ZIbYYhVmBrvqG+aBUz33\n\tY24uu3zJAbn3xFNdwGlYQYb//LofaPkjMYM2YF/tSoNUyavBC6qSeX7oC07PcNpt07zj\n\tjqb97q8QCqFetVHbSEB8I6SJlH4/cyk43ieMWFHQm4lDx/805ttdYAJoEWQozD2O4ZbN\n\t7I3yOnSquQZHyP3GomVFi4E7fPK6bvQx5MGwRom6wBEJBvuTN64AMwRsSx4pDHsBi3C2\n\tf6kaNMa7FnOtlHd+fARhWjWdaFbvCeoFmoCO6S9yY6laqCjME0wzvZ9+DilnWqLwFLEt\n\teBOg==","X-Gm-Message-State":"AKGB3mKOZhd4usUnLnRKlSmXeOwf6gS8MqYeaC4G5P6FBecZ87cen55i\n\tQrPvqQXCxXyeWAug30rP7MBFavXvbp5si62NzsCEzQ==","X-Google-Smtp-Source":"AGs4zMakcKMz7dwWhiEXyMTyczaMfzJiTsSfIBXFy5YdUJldQAjy2uvkxXzUjhJ0ISZUAYCgzMMtGzyaglMIqzSphTI=","X-Received":"by 10.55.201.11 with SMTP id q11mr19986071qki.303.1512427524353; \n\tMon, 04 Dec 2017 14:45:24 -0800 (PST)","MIME-Version":"1.0","In-Reply-To":"<3949db75-20a0-33d2-e879-3a42ea0d85ce@gmx.de>","References":"<20170917225927.117917-1-sjg@chromium.org>\n\t<20170917225927.117917-4-sjg@chromium.org>\n\t<3949db75-20a0-33d2-e879-3a42ea0d85ce@gmx.de>","From":"Simon Glass ","Date":"Mon, 4 Dec 2017 15:45:03 -0700","X-Google-Sender-Auth":"bvA6frVBVGh5d2Q4zuu6cGjb51Q","Message-ID":"","To":"Heinrich Schuchardt ","Cc":"U-Boot Mailing List ,\n\tAlexander Graf , \n\tAndy Shevchenko ","Subject":"Re: [U-Boot] [PATCH 03/16] efi: Add error checking for\n\tefi_init_obj_list()","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}}]