Re: [PATCH v11 0/1] ShellPkg/DynamicCommand: add HttpDynamicCommand


Vladimir Olovyannikov
 

Hi Laszlo,

-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com>
Sent: Wednesday, September 9, 2020 11:33 PM
To: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>;
devel@edk2.groups.io
Cc: Zhichao Gao <zhichao.gao@intel.com>; Maciej Rabeda
<maciej.rabeda@linux.intel.com>; Jiaxin Wu <jiaxin.wu@intel.com>; Siyuan
Fu <siyuan.fu@intel.com>; Ray Ni <ray.ni@intel.com>; Liming Gao
<liming.gao@intel.com>; Nd <nd@arm.com>; Samer El-Haj-Mahmoud
<Samer.El-Haj-Mahmoud@arm.com>
Subject: Re: [PATCH v11 0/1] ShellPkg/DynamicCommand: add
HttpDynamicCommand

On 09/09/20 20:49, Vladimir Olovyannikov wrote:
Signed-off-by: Vladimir Olovyannikov
<vladimir.olovyannikov@broadcom.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Nd <nd@arm.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

This patchset introduces 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

PATCH v11 changes:
Address comments from Laszlo:
- use TimeBaseLib.h header to get rid of duplicated constants;
- explicitly return UINT32 in EfiTimeToEpoch().
to be clear, I explicitly *disagree* with returning UINT32 from
EfiTimeToEpoch().

I'm not "demanding" (or even suggesting) that you update the
EfiTimeToEpoch() implementation in this patch to return UINTN, but I'd
like
to be very clear that, IMO, for EfiTimeToEpoch() to suffer from a year
2106
problem on 64-bit systems too, is bad design. So please don't list the
UINT32
return type as my suggestion -- that's the exact opposite of what I'd
actually
suggest.
Sorry, I must have misunderstood. Do you want me to resubmit the patch?
I am open to ideas.

Thank you,
Vladimir

Zhichao: are you ready to merge this patch? If so, please let me know;
I'll test
it then.

Thanks
Laszlo



Vladimir Olovyannikov (1):
ShellPkg/DynamicCommand: add HttpDynamicCommand

ShellPkg/ShellPkg.dec | 1 +
ShellPkg/ShellPkg.dsc | 5 +
.../HttpDynamicCommand/HttpApp.inf | 58 +
.../HttpDynamicCommand/HttpDynamicCommand.inf | 63 +
.../DynamicCommand/HttpDynamicCommand/Http.h | 91 +
ShellPkg/Include/Guid/ShellLibHiiGuid.h | 5 +
.../DynamicCommand/HttpDynamicCommand/Http.c | 1823
+++++++++++++++++
.../HttpDynamicCommand/HttpApp.c | 61 +
.../HttpDynamicCommand/HttpDynamicCommand.c | 137 ++
.../HttpDynamicCommand/Http.uni | 117 ++
10 files changed, 2361 insertions(+)
create mode 100644
ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf
create mode 100644
ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand
.inf
create mode 100644
ShellPkg/DynamicCommand/HttpDynamicCommand/Http.h
create mode 100644
ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c
create mode 100644
ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.c
create mode 100644
ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand
.c
create mode 100644
ShellPkg/DynamicCommand/HttpDynamicCommand/Http.uni

Join devel@edk2.groups.io to automatically receive all group messages.