Re: [edk2-devel] [PATCH v6 1/1] ShellPkg/DynamicCommand: add HttpDynamicCommand

["Vladimir Olovyannikov" <vladimir.olovyannikov@broadcom.com>]

Hi Maciej,

Thank you for spotting these.

> -----Original Message-----
> From: Rabeda, Maciej <maciej.rabeda@linux.intel.com>
> Sent: Tuesday, August 25, 2020 4:56 AM
> To: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>; Laszlo
> Ersek <lersek@redhat.com>; devel@edk2.groups.io
> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Zhichao
> Gao <zhichao.gao@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>
> Subject: Re: [edk2-devel] [PATCH v6 1/1] ShellPkg/DynamicCommand: add
> HttpDynamicCommand
>
> Hi Vladimir,
>
> Since you are already resending, could you address very minor coding
> standard items?
>
> HttpDynamicCommand.c
> Line 60: space after func name call
>
> Http.c
> Line 519: indent
> Line 586: ); alignment
> Line 879: space between SetTimer and (
> Line 1579: ); to next line
>
> Upon removing openssl and addressing the items above, you can tie:
> Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
OK, sure, thanks . I've addressed those and am going to submit v7 patchset.
Just wondering if you use any tool other than the python script
PatchCheck.py under BaseTools/Scripts?
I ran that script before, and it found no issues. If there is a better way
please let me know.

Thank you,
Vladimir
>
> Thanks,
> Maciej
>
> On 25-Aug-20 06:16, Vladimir Olovyannikov wrote:
> > Hi Laszlo,
> >
> > Sorry, I should've checked. No, there was no intention at all to even
> > touch an OpenSSL lib.
> > I will re-send tomorrow morning.
> >
> > Thank you,
> > Vladimir
> >
> >> -----Original Message-----
> >> From: Laszlo Ersek <lersek@redhat.com>
> >> Sent: Monday, August 24, 2020 12:43 PM
> >> To: devel@edk2.groups.io; vladimir.olovyannikov@broadcom.com
> >> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>;
> 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>
> >> Subject: Re: [edk2-devel] [PATCH v6 1/1] ShellPkg/DynamicCommand:
> add
> >> HttpDynamicCommand
> >>
> >> Hi Vladimir,
> >>
> >> On 08/21/20 18:05, 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
> >>> <vladimir.olovyannikov@broadcom.com>
> >>> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> >>> Cc: Laszlo Ersek <lersek@redhat.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>
> >>> ---
> >>>   ShellPkg/ShellPkg.dec                         |    1 +
> >>>   ShellPkg/ShellPkg.dsc                         |    5 +
> >>>   .../HttpDynamicCommand/HttpApp.inf            |   58 +
> >>>   .../HttpDynamicCommand/HttpDynamicCommand.inf |   63 +
> >>>   .../DynamicCommand/HttpDynamicCommand/Http.h  |   88 +
> >>>   ShellPkg/Include/Guid/ShellLibHiiGuid.h       |    5 +
> >>>   .../DynamicCommand/HttpDynamicCommand/Http.c  | 1693
> >> +++++++++++++++++
> >>>   .../HttpDynamicCommand/HttpApp.c              |   61 +
> >>>   .../HttpDynamicCommand/HttpDynamicCommand.c   |  137 ++
> >>>   CryptoPkg/Library/OpensslLib/openssl          |    2 +-
> >>>   .../HttpDynamicCommand/Http.uni               |  116 ++
> >>>   11 files changed, 2228 insertions(+), 1 deletion(-)  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
> >> [...]
> >>
> >>> diff --git a/CryptoPkg/Library/OpensslLib/openssl
> >>> b/CryptoPkg/Library/OpensslLib/openssl
> >>> index e2e09d9fba11..c3656cc594da 160000
> >>> --- a/CryptoPkg/Library/OpensslLib/openssl
> >>> +++ b/CryptoPkg/Library/OpensslLib/openssl
> >>> @@ -1 +1 @@
> >>> -Subproject commit e2e09d9fba1187f8d6aafaa34d4172f56f1ffb72
> >>> +Subproject commit c3656cc594daac8167721dde7220f0e59ae146fc
> >> your patch includes a hunk that moves our openssl submodule from
> >> 1.1.1g back to ~1.1.1d.
> >>
> >> I think that must be unintentional; can you please drop this hunk and
> >> post v7?
> >>
> >> Thanks
> >> Laszlo