From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lj1-f175.google.com (mail-lj1-f175.google.com [209.85.208.175]) by mx.groups.io with SMTP id smtpd.web10.10682.1596563015879260908 for ; Tue, 04 Aug 2020 10:43:36 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@broadcom.com header.s=google header.b=NFkEqDLp; spf=permerror, err=parse error for token &{10 18 %{i}._ip.%{h}._ehlo.%{d}._spf.vali.email}: invalid domain name (domain: broadcom.com, ip: 209.85.208.175, mailfrom: vladimir.olovyannikov@broadcom.com) Received: by mail-lj1-f175.google.com with SMTP id 185so34329285ljj.7 for ; Tue, 04 Aug 2020 10:43:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=from:references:in-reply-to:mime-version:thread-index:date :message-id:subject:to:cc; bh=6EPIY4/ijTqFWuic8HKSNGhuV7JQTvH/SwXi5+6trAw=; b=NFkEqDLprnOsrp1R9A8JiHzfCTmwVLg+RxT9bws3UDyUIFsJioXETydZk5xV+OoAxr G0oeqtObORZmMDStdw/lLw1WCFoNCH58Q0jkMqssyaS0ixo73eFJfnj2sxyG97Xq58IS 12uNYeUWAgKEICuqnFpOHfES6cCrwSC4jpI48= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:references:in-reply-to:mime-version :thread-index:date:message-id:subject:to:cc; bh=6EPIY4/ijTqFWuic8HKSNGhuV7JQTvH/SwXi5+6trAw=; b=SKrEtwCE3h6AaJTvIiZE9tqtcnUH44FUy8q6cc0Rr+/CfIkEJRvThQVg9JDIxSMr1x ZkhUpBWnLV2QCd4Yhz3E8SpnKPcw+SwiwRhQ3sRx1iZRGs9s4pKKbDp+5GY60kVXi7Lq GhMJOEPizrHrSgjBYCYKdIv95WjUH/gO44VK2v6Np/MyUon5U3pXSo790aEvDnzC/oJL 34VtjjxKr7AkxfhGAUxL4ol5pdgCT+8GuaEC9dNs6Nc8ERo4BKQvtXBb/xknwkv2/GrH U54zcGQhCmAiHn3MYAqfq/o3i3JvSJKgD9LPgNqtghFhnpu9Y0Mn7CqbakC6+5N0JKwP NV3w== X-Gm-Message-State: AOAM530rr9fWYnltiX4e2Up5zQgznT/Dw7Vhf2envA7tFGy0Sg9Sja06 AS4y6WMfrc96/0o+fcuXO8oEKzlFS+zLUVnegP6SqRr1uzVWgg== X-Google-Smtp-Source: ABdhPJxFy7cpRcJv6UeIXwuGNz/j5MsIzAMjYgswGBdDqelwm4FRMDcO6SO/q8RAIgBCMHz2VheRvFoUk08N5XWt4Ug= X-Received: by 2002:a2e:9cd3:: with SMTP id g19mr7692022ljj.229.1596563013404; Tue, 04 Aug 2020 10:43:33 -0700 (PDT) From: "Vladimir Olovyannikov" References: <20200727164830.25829-1-vladimir.olovyannikov@broadcom.com> <20200727164830.25829-2-vladimir.olovyannikov@broadcom.com> In-Reply-To: MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQILIYHslp9C8beoIeXBb+C1khe8nAJZRV/xAb0dpCaonhb7kA== Date: Tue, 4 Aug 2020 10:43:32 -0700 Message-ID: Subject: Re: [edk2-devel] [PATCH v5 1/1] ShellPkg/DynamicCommand: add HttpDynamicCommand To: devel@edk2.groups.io, lersek@redhat.com Cc: Samer El-Haj-Mahmoud , Zhichao Gao , Maciej Rabeda , Jiaxin Wu , Siyuan Fu , Ray Ni , Liming Gao , Nd Content-Type: text/plain; charset="UTF-8" Hi Laszlo, Thank you for the comments. I agree with them, and as you suggest, I will address them along with comments from maintainers. Thank you, Vladimir > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Laszlo > Ersek > Sent: Monday, July 27, 2020 3:08 PM > To: devel@edk2.groups.io; vladimir.olovyannikov@broadcom.com > Cc: Samer El-Haj-Mahmoud ; Zhichao > Gao ; Maciej Rabeda > ; Jiaxin Wu ; Siyuan > Fu ; Ray Ni ; Liming Gao > ; Nd > Subject: Re: [edk2-devel] [PATCH v5 1/1] ShellPkg/DynamicCommand: add > HttpDynamicCommand > > Just some quick remarks after a comparison with v3: > > On 07/27/20 18:48, Vladimir Olovyannikov via groups.io wrote: > > Introduce an http client utilizing EDK2 HTTP protocol, to allow fast > > image downloading from http/https servers. > > HTTP download speed is usually faster than tftp. > > The client is based on the same approach as tftp dynamic command, and > > uses the same UEFI Shell command line parameters. This makes it easy > > integrating http into existing UEFI Shell scripts. > > Note that to enable HTTP download, feature Pcd > > gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections must be set to > > TRUE. > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2860 > > > > Signed-off-by: Vladimir Olovyannikov > > > > CC: Samer El-Haj-Mahmoud > > CC: Laszlo Ersek > > (1) These "CC" lines are not formatted correctly -- they might do the job > as > far as git-send-email is concerned, but they don't satisfy > "PatchCheck.py": > > > ShellPkg/DynamicCommand: add HttpDynamicCommand The commit > message > > format is not valid: > > * 'CC' should be 'Cc' > > * 'CC' should be 'Cc' > > https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-F > > ormat > > The exit status is 255, so this would break the CI run again. > > Please run "PatchCheck.py" locally before posting, and/or submit a > personal > CI build. > > [...] > > > + if (UserNicName != NULL) { > > + if (StrCmp (NicName, UserNicName) != 0) { > > + Status = EFI_NOT_FOUND; > > Change is new since v4, but not documented in the v5 changelog. > > (The code may be OK, but please help reviewers with the v(n) -> v(n+1) > changelogs.) > > [...] > > > + if (ShellCommandLineGetFlag (CheckPackage, L"-m")) { > > + Context.Flags |= DL_FLAG_TIME; > > + } > > (2) The "-m" flag has not been removed from here > > [...] > > > +// Download Flags > > +#define DL_FLAG_TIME BIT0 // Show elapsed time. > > (3) and here > > > +".SH SYNOPSIS\r\n" > > +" \r\n" > > +"HTTP [-i interface] [-l port] [-t timeout] [-s size] [-m] [-k]\r\n" > > (4) and here > > [...] > > > +" -m Measure and report download time (in seconds). > > \r\n" > > (5) and here. > > I suggest waiting for ShellPkg owner feedback before posting v6. > > Thanks! > Laszlo > > >