Thanks Laszlo, I have refined the patch to v2 with your "Reviewed-by" tag.
> -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Wednesday, August 1, 2018 5:49 PM > To: Wu, Jiaxin <jiaxin...@intel.com>; edk2-devel@lists.01.org > Cc: Ye, Ting <ting...@intel.com>; Fu, Siyuan <siyuan...@intel.com> > Subject: Re: [Patch] NetworkPkg/HttpDxe: Stripped square brackets in IPv6 > expressed HostName. > > On 08/01/18 03:54, Jiaxin Wu wrote: > > In URI, the colon (:) is used to terminate the HostName path before > > a port number. However, if HostName is expressed as IPv6 format, colon > > characters in IPv6 addresses will conflict with the colon before port > > number. To alleviate this conflict in URI, the IPv6 expressed HostName > > are enclosed in square brackets ([]). To record the real IPv6 HostName, > > square brackets should be stripped. > > > > Cc: Ye Ting <ting...@intel.com> > > Cc: Fu Siyuan <siyuan...@intel.com> > > Cc: Laszlo Ersek <ler...@redhat.com> > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Wu Jiaxin <jiaxin...@intel.com> > > --- > > NetworkPkg/HttpDxe/HttpImpl.c | 17 ++++++++++++++--- > > 1 file changed, 14 insertions(+), 3 deletions(-) > > > > diff --git a/NetworkPkg/HttpDxe/HttpImpl.c > b/NetworkPkg/HttpDxe/HttpImpl.c > > index 17deceb395..e05ee9344b 100644 > > --- a/NetworkPkg/HttpDxe/HttpImpl.c > > +++ b/NetworkPkg/HttpDxe/HttpImpl.c > > @@ -403,14 +403,25 @@ EfiHttpRequest ( > > Status = HttpParseUrl (Url, (UINT32) AsciiStrLen (Url), FALSE, > > &UrlParser); > > if (EFI_ERROR (Status)) { > > goto Error1; > > } > > > > - HostName = NULL; > > - Status = HttpUrlGetHostName (Url, UrlParser, &HostName); > > + Status = HttpUrlGetHostName (Url, UrlParser, &HostName); > > if (EFI_ERROR (Status)) { > > - goto Error1; > > + goto Error1; > > + } > > + > > + if (HttpInstance->LocalAddressIsIPv6 && AsciiStrSize (HostName) > 2 > && > > + HostName[0] == '[' && *(HostName + (AsciiStrSize (HostName) - 2)) > == ']') { > > + // > > + // HostName format is expressed as IPv6, so, remove '[' and ']'. > > + // > > + HostNameSize = AsciiStrSize (HostName) - 2; > > + > > + CopyMem (HostName, HostName + 1, HostNameSize - 1); > > + > > + *(HostName + HostNameSize - 1) = '\0'; > > } > > > > Status = HttpUrlGetPort (Url, UrlParser, &RemotePort); > > if (EFI_ERROR (Status)) { > > if (HttpInstance->UseHttps) { > > > > There are a number of expressions of the form > > *(HostName + Offset) > > which could be rewritten more idiomatically as > > HostName[Offset] > > In addition, I think the code could be optimized by calculating > AsciiStrSize() only once: > > if (HttpInstance->LocalAddressIsIPv6) { > HostNameSize = AsciiStrSize (HostName); > > if (HostNameSize > 2 && > HostName[0] == '[' && > HostName[HostNameSize - 2] == ']') { > // > // HostName format is expressed as IPv6, so, remove '[' and ']'. > // > HostNameSize -= 2; > CopyMem (HostName, HostName + 1, HostNameSize - 1); > HostName[HostNameSize - 1] = '\0'; > } > } > > Under my proposal, if the inner condition fails, then "HostNameSize" > will be set as a "side effect", but I don't think that's a problem. > > > Anyway, the patch seems technically correct; if you don't want to submit > a v2, I'm fine with this variant too: > > Reviewed-by: Laszlo Ersek <ler...@redhat.com> > > I have just one request for the subject line, before you push the patch: > please replace "stripped" with "strip". > > Thanks! > Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
