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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9D228C433EF for ; Tue, 9 Nov 2021 08:11:40 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 243F86112D for ; Tue, 9 Nov 2021 08:11:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 243F86112D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 02E4A8392A; Tue, 9 Nov 2021 09:11:34 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com 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=gmail.com header.i=@gmail.com header.b="R+AVrmB0"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 7D1AD8393A; Tue, 9 Nov 2021 09:11:30 +0100 (CET) Received: from mail-oi1-x22c.google.com (mail-oi1-x22c.google.com [IPv6:2607:f8b0:4864:20::22c]) (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 8199583925 for ; Tue, 9 Nov 2021 09:11:24 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=rfried.dev@gmail.com Received: by mail-oi1-x22c.google.com with SMTP id o4so32374661oia.10 for ; Tue, 09 Nov 2021 00:11:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=r6TkTY7sf6gaZKOvtGzU9iA446Es7QjDwxOtM1h4u94=; b=R+AVrmB0wf0wx+4L/Qhh6xAdDYe+UDUTfV3GuwTCc4gbWiR1ahwFCg5vQkIZnO4v5w D/rVNam6cJo7tb0i+v4CLQI2kCL8x0H5u7pJK9/5UoVp8nzNtA5uoI/G5nD3MF56YffM k6VVtbDi/5D4LgcQqXevrOeSJf82eMkpbjvfk4PsliOaL83Dhu7jRA7edMnHecWVepw8 41PkxIIchC4ycA3tfxD0jweRKjMg5yZplRvARukEL7j09CSY32pNKCGeFWwLFc/0HdHL SHwspDISf0eDmm3FyKaCgp6iLP6jRrBF60kddcwQ5n911oscc/5jxM9h+i7ADJtnyva1 EU6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=r6TkTY7sf6gaZKOvtGzU9iA446Es7QjDwxOtM1h4u94=; b=5Xf10I1oFKo/uP//d/ZgyIyNUAesG05rH684ah//5gfRednpqEabXR34O6XnfVJVbC 2bZJWjelgZCxf0bkMtrs0qa7cyFzT0yM2+V67TdPe87ydPLQJfjC4gbZ6mBt5yX/V1xe LeXsytnJwQd1Q/MRcrHnsUmaVqZOed7uxbXsI0fWkOmUaqCaWJUcBn0DlxY8q66ESb7+ WOjPGoIjRfO4WKKFFgF5e+hauCaeHxk00lNjkVztoMMfy/BPN3vAWL1OWORi2tFaDG++ UneEddE1xjo0Mx6pjwKDV8LJhJXxQim2vIm8MKgEPfl312keS911905xowxhde8Vdycs 4CYg== X-Gm-Message-State: AOAM533eV1l19jJhOGI/hWYH0tgQd+ju9JMOJllRGcYZXSI1IgwdwzPX hn+k6R7s6V2BE9A5Xqt/NPhkeE3krhQMj8H/ebA= X-Google-Smtp-Source: ABdhPJxU2l9VK63n2CdjtNKJbEVVsJ+ISEgDfDHd8iczgzH9mx1otPSKtrjvloQTBq7qgM1eAoCy/QgPHK/970AjCmA= X-Received: by 2002:a05:6808:1914:: with SMTP id bf20mr1936945oib.7.1636445483219; Tue, 09 Nov 2021 00:11:23 -0800 (PST) MIME-Version: 1.0 References: <20211014184811.482560-1-sjg@chromium.org> <20211014184811.482560-5-sjg@chromium.org> In-Reply-To: <20211014184811.482560-5-sjg@chromium.org> From: Ramon Fried Date: Tue, 9 Nov 2021 10:11:11 +0200 Message-ID: Subject: Re: [PATCH v3 04/18] pxe: Move do_getfile() into the context To: Simon Glass Cc: U-Boot Mailing List , Patrice Chotard , Artem Lapkin , Tom Rini , Joe Hershberger , Heinrich Schuchardt , Peter Hoyes Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 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.2 at phobos.denx.de X-Virus-Status: Clean On Thu, Oct 14, 2021 at 9:49 PM Simon Glass wrote: > > Rather than having a global variable, pass the function as part of the > context. > > Signed-off-by: Simon Glass > --- > > (no changes since v1) > > cmd/pxe.c | 10 ++++------ > cmd/pxe_utils.c | 9 ++++----- > cmd/pxe_utils.h | 20 +++++++++++++++++--- > cmd/sysboot.c | 20 ++++++++++---------- > 4 files changed, 35 insertions(+), 24 deletions(-) > > diff --git a/cmd/pxe.c b/cmd/pxe.c > index 17ce54fc049..70dbde3a636 100644 > --- a/cmd/pxe.c > +++ b/cmd/pxe.c > @@ -24,7 +24,7 @@ const char *pxe_default_paths[] = { > NULL > }; > > -static int do_get_tftp(struct cmd_tbl *cmdtp, const char *file_path, > +static int do_get_tftp(struct pxe_context *ctx, const char *file_path, > char *file_addr) > { > char *tftp_argv[] = {"tftp", NULL, NULL, NULL}; > @@ -32,7 +32,7 @@ static int do_get_tftp(struct cmd_tbl *cmdtp, const char *file_path, > tftp_argv[1] = file_addr; > tftp_argv[2] = (void *)file_path; > > - if (do_tftpb(cmdtp, 0, 3, tftp_argv)) > + if (do_tftpb(ctx->cmdtp, 0, 3, tftp_argv)) > return -ENOENT; > > return 1; > @@ -121,8 +121,7 @@ do_pxe_get(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > struct pxe_context ctx; > int err, i = 0; > > - pxe_setup_ctx(&ctx, cmdtp); > - do_getfile = do_get_tftp; > + pxe_setup_ctx(&ctx, cmdtp, do_get_tftp); > > if (argc != 1) > return CMD_RET_USAGE; > @@ -176,8 +175,7 @@ do_pxe_boot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > char *pxefile_addr_str; > struct pxe_context ctx; > > - pxe_setup_ctx(&ctx, cmdtp); > - do_getfile = do_get_tftp; > + pxe_setup_ctx(&ctx, cmdtp, do_get_tftp); > > if (argc == 1) { > pxefile_addr_str = from_env("pxefile_addr_r"); > diff --git a/cmd/pxe_utils.c b/cmd/pxe_utils.c > index 280be55d9b3..2caae6d1555 100644 > --- a/cmd/pxe_utils.c > +++ b/cmd/pxe_utils.c > @@ -94,9 +94,6 @@ static int get_bootfile_path(const char *file_path, char *bootfile_path, > return 1; > } > > -int (*do_getfile)(struct cmd_tbl *cmdtp, const char *file_path, > - char *file_addr); > - > /* > * As in pxelinux, paths to files referenced from files we retrieve are > * relative to the location of bootfile. get_relfile takes such a path and > @@ -133,7 +130,7 @@ static int get_relfile(struct pxe_context *ctx, const char *file_path, > > sprintf(addr_buf, "%lx", file_addr); > > - return do_getfile(ctx->cmdtp, relfile, addr_buf); > + return ctx->getfile(ctx, relfile, addr_buf); > } > > int get_pxe_file(struct pxe_context *ctx, const char *file_path, > @@ -1434,7 +1431,9 @@ void handle_pxe_menu(struct pxe_context *ctx, struct pxe_menu *cfg) > boot_unattempted_labels(ctx, cfg); > } > > -void pxe_setup_ctx(struct pxe_context *ctx, struct cmd_tbl *cmdtp) > +void pxe_setup_ctx(struct pxe_context *ctx, struct cmd_tbl *cmdtp, > + pxe_getfile_func getfile) > { > ctx->cmdtp = cmdtp; > + ctx->getfile = getfile; > } > diff --git a/cmd/pxe_utils.h b/cmd/pxe_utils.h > index cd0d3371765..ca2696f48b0 100644 > --- a/cmd/pxe_utils.h > +++ b/cmd/pxe_utils.h > @@ -77,16 +77,28 @@ struct pxe_menu { > > extern bool is_pxe; > > -extern int (*do_getfile)(struct cmd_tbl *cmdtp, const char *file_path, > - char *file_addr); > +struct pxe_context; > +typedef int (*pxe_getfile_func)(struct pxe_context *ctx, const char *file_path, > + char *file_addr); > > /** > * struct pxe_context - context information for PXE parsing > * > * @cmdtp: Pointer to command table to use when calling other commands > + * @getfile: Function called by PXE to read a file > */ > struct pxe_context { > struct cmd_tbl *cmdtp; > + /** > + * getfile() - read a file > + * > + * @ctx: PXE context > + * @file_path: Path to the file > + * @file_addr: String containing the hex address to put the file in > + * memory > + * Return 0 if OK, -ve on error > + */ > + pxe_getfile_func getfile; > }; > > /** > @@ -179,7 +191,9 @@ int format_mac_pxe(char *outbuf, size_t outbuf_len); > * > * @ctx: Context to set up > * @cmdtp: Command table entry which started this action > + * @getfile: Function to call to read a file > */ > -void pxe_setup_ctx(struct pxe_context *ctx, struct cmd_tbl *cmdtp); > +void pxe_setup_ctx(struct pxe_context *ctx, struct cmd_tbl *cmdtp, > + pxe_getfile_func getfile); > > #endif /* __PXE_UTILS_H */ > diff --git a/cmd/sysboot.c b/cmd/sysboot.c > index 9ba713c8aae..082f23543d1 100644 > --- a/cmd/sysboot.c > +++ b/cmd/sysboot.c > @@ -8,7 +8,7 @@ > > static char *fs_argv[5]; > > -static int do_get_ext2(struct cmd_tbl *cmdtp, const char *file_path, > +static int do_get_ext2(struct pxe_context *ctx, const char *file_path, > char *file_addr) > { > #ifdef CONFIG_CMD_EXT2 > @@ -16,13 +16,13 @@ static int do_get_ext2(struct cmd_tbl *cmdtp, const char *file_path, > fs_argv[3] = file_addr; > fs_argv[4] = (void *)file_path; > > - if (!do_ext2load(cmdtp, 0, 5, fs_argv)) > + if (!do_ext2load(ctx->cmdtp, 0, 5, fs_argv)) > return 1; > #endif > return -ENOENT; > } > > -static int do_get_fat(struct cmd_tbl *cmdtp, const char *file_path, > +static int do_get_fat(struct pxe_context *ctx, const char *file_path, > char *file_addr) > { > #ifdef CONFIG_CMD_FAT > @@ -30,13 +30,13 @@ static int do_get_fat(struct cmd_tbl *cmdtp, const char *file_path, > fs_argv[3] = file_addr; > fs_argv[4] = (void *)file_path; > > - if (!do_fat_fsload(cmdtp, 0, 5, fs_argv)) > + if (!do_fat_fsload(ctx->cmdtp, 0, 5, fs_argv)) > return 1; > #endif > return -ENOENT; > } > > -static int do_get_any(struct cmd_tbl *cmdtp, const char *file_path, > +static int do_get_any(struct pxe_context *ctx, const char *file_path, > char *file_addr) > { > #ifdef CONFIG_CMD_FS_GENERIC > @@ -44,7 +44,7 @@ static int do_get_any(struct cmd_tbl *cmdtp, const char *file_path, > fs_argv[3] = file_addr; > fs_argv[4] = (void *)file_path; > > - if (!do_load(cmdtp, 0, 5, fs_argv, FS_TYPE_ANY)) > + if (!do_load(ctx->cmdtp, 0, 5, fs_argv, FS_TYPE_ANY)) > return 1; > #endif > return -ENOENT; > @@ -91,13 +91,13 @@ static int do_sysboot(struct cmd_tbl *cmdtp, int flag, int argc, > env_set("bootfile", filename); > } > > - pxe_setup_ctx(&ctx, cmdtp); > + pxe_setup_ctx(&ctx, cmdtp, NULL); > if (strstr(argv[3], "ext2")) { > - do_getfile = do_get_ext2; > + ctx.getfile = do_get_ext2; > } else if (strstr(argv[3], "fat")) { > - do_getfile = do_get_fat; > + ctx.getfile = do_get_fat; > } else if (strstr(argv[3], "any")) { > - do_getfile = do_get_any; > + ctx.getfile = do_get_any; > } else { > printf("Invalid filesystem: %s\n", argv[3]); > return 1; > -- > 2.33.0.1079.g6e70778dc9-goog > Reviewed-by: Ramon Fried