public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch] NetworkPkg/HttpDxe: Stripped square brackets in IPv6 expressed HostName.
@ 2018-08-01  1:54 Jiaxin Wu
  2018-08-01  2:05 ` Fu, Siyuan
  2018-08-01  9:49 ` Laszlo Ersek
  0 siblings, 2 replies; 6+ messages in thread
From: Jiaxin Wu @ 2018-08-01  1:54 UTC (permalink / raw)
  To: edk2-devel; +Cc: Ye Ting, Fu Siyuan, Laszlo Ersek, Wu Jiaxin

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.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@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) {
-- 
2.17.1.windows.2



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Patch] NetworkPkg/HttpDxe: Stripped square brackets in IPv6 expressed HostName.
  2018-08-01  1:54 [Patch] NetworkPkg/HttpDxe: Stripped square brackets in IPv6 expressed HostName Jiaxin Wu
@ 2018-08-01  2:05 ` Fu, Siyuan
  2018-08-01  2:42   ` Wu, Jiaxin
  2018-08-01  9:49 ` Laszlo Ersek
  1 sibling, 1 reply; 6+ messages in thread
From: Fu, Siyuan @ 2018-08-01  2:05 UTC (permalink / raw)
  To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Ye, Ting, Laszlo Ersek

Hi, Jiaxin

The HttpLib already has HttpUrlGetIp6() for this.

BestRegards
Fu Siyuan

> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Wednesday, August 1, 2018 9:55 AM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [Patch] NetworkPkg/HttpDxe: Stripped square brackets in IPv6
> expressed HostName.
> 
> 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.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@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) {
> --
> 2.17.1.windows.2



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch] NetworkPkg/HttpDxe: Stripped square brackets in IPv6 expressed HostName.
  2018-08-01  2:05 ` Fu, Siyuan
@ 2018-08-01  2:42   ` Wu, Jiaxin
  2018-08-01  3:22     ` Fu, Siyuan
  0 siblings, 1 reply; 6+ messages in thread
From: Wu, Jiaxin @ 2018-08-01  2:42 UTC (permalink / raw)
  To: Fu, Siyuan, edk2-devel@lists.01.org; +Cc: Ye, Ting, Laszlo Ersek

Hi Siyuan,

Even we have one lib for us to get the IPv6 address, I still prefer to use the patch did, because the format returned from the HttpUrlGetIp6 is EFI_IPv6_ADDRESS, we have to transform it to CHAR8, which means we also need another lib included here to do that. Actually, the patch is quite simply even than API did. What do you think?

Thanks,
Jiaxin

> -----Original Message-----
> From: Fu, Siyuan
> Sent: Wednesday, August 1, 2018 10:06 AM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: RE: [Patch] NetworkPkg/HttpDxe: Stripped square brackets in IPv6
> expressed HostName.
> 
> Hi, Jiaxin
> 
> The HttpLib already has HttpUrlGetIp6() for this.
> 
> BestRegards
> Fu Siyuan
> 
> > -----Original Message-----
> > From: Wu, Jiaxin
> > Sent: Wednesday, August 1, 2018 9:55 AM
> > To: edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Laszlo
> > Ersek <lersek@redhat.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> > Subject: [Patch] NetworkPkg/HttpDxe: Stripped square brackets in IPv6
> > expressed HostName.
> >
> > 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.ye@intel.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Wu Jiaxin <jiaxin.wu@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) {
> > --
> > 2.17.1.windows.2



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch] NetworkPkg/HttpDxe: Stripped square brackets in IPv6 expressed HostName.
  2018-08-01  2:42   ` Wu, Jiaxin
@ 2018-08-01  3:22     ` Fu, Siyuan
  0 siblings, 0 replies; 6+ messages in thread
From: Fu, Siyuan @ 2018-08-01  3:22 UTC (permalink / raw)
  To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Ye, Ting, Laszlo Ersek

OK, the patch is good with me.

Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>


> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Wednesday, August 1, 2018 10:42 AM
> To: Fu, Siyuan <siyuan.fu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: RE: [Patch] NetworkPkg/HttpDxe: Stripped square brackets in IPv6
> expressed HostName.
> 
> Hi Siyuan,
> 
> Even we have one lib for us to get the IPv6 address, I still prefer to use
> the patch did, because the format returned from the HttpUrlGetIp6 is
> EFI_IPv6_ADDRESS, we have to transform it to CHAR8, which means we also
> need another lib included here to do that. Actually, the patch is quite
> simply even than API did. What do you think?
> 
> Thanks,
> Jiaxin
> 
> > -----Original Message-----
> > From: Fu, Siyuan
> > Sent: Wednesday, August 1, 2018 10:06 AM
> > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>; Laszlo Ersek <lersek@redhat.com>
> > Subject: RE: [Patch] NetworkPkg/HttpDxe: Stripped square brackets in
> IPv6
> > expressed HostName.
> >
> > Hi, Jiaxin
> >
> > The HttpLib already has HttpUrlGetIp6() for this.
> >
> > BestRegards
> > Fu Siyuan
> >
> > > -----Original Message-----
> > > From: Wu, Jiaxin
> > > Sent: Wednesday, August 1, 2018 9:55 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>;
> Laszlo
> > > Ersek <lersek@redhat.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> > > Subject: [Patch] NetworkPkg/HttpDxe: Stripped square brackets in IPv6
> > > expressed HostName.
> > >
> > > 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.ye@intel.com>
> > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Wu Jiaxin <jiaxin.wu@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) {
> > > --
> > > 2.17.1.windows.2



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch] NetworkPkg/HttpDxe: Stripped square brackets in IPv6 expressed HostName.
  2018-08-01  1:54 [Patch] NetworkPkg/HttpDxe: Stripped square brackets in IPv6 expressed HostName Jiaxin Wu
  2018-08-01  2:05 ` Fu, Siyuan
@ 2018-08-01  9:49 ` Laszlo Ersek
  2018-08-02  1:24   ` Wu, Jiaxin
  1 sibling, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2018-08-01  9:49 UTC (permalink / raw)
  To: Jiaxin Wu, edk2-devel; +Cc: Ye Ting, Fu Siyuan

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.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@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 <lersek@redhat.com>

I have just one request for the subject line, before you push the patch:
please replace "stripped" with "strip".

Thanks!
Laszlo


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch] NetworkPkg/HttpDxe: Stripped square brackets in IPv6 expressed HostName.
  2018-08-01  9:49 ` Laszlo Ersek
@ 2018-08-02  1:24   ` Wu, Jiaxin
  0 siblings, 0 replies; 6+ messages in thread
From: Wu, Jiaxin @ 2018-08-02  1:24 UTC (permalink / raw)
  To: Laszlo Ersek, edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan

Thanks Laszlo, I have refined the patch to v2 with your "Reviewed-by" tag.



> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Wednesday, August 1, 2018 5:49 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@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.ye@intel.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Wu Jiaxin <jiaxin.wu@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 <lersek@redhat.com>
> 
> I have just one request for the subject line, before you push the patch:
> please replace "stripped" with "strip".
> 
> Thanks!
> Laszlo

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-08-02  1:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-01  1:54 [Patch] NetworkPkg/HttpDxe: Stripped square brackets in IPv6 expressed HostName Jiaxin Wu
2018-08-01  2:05 ` Fu, Siyuan
2018-08-01  2:42   ` Wu, Jiaxin
2018-08-01  3:22     ` Fu, Siyuan
2018-08-01  9:49 ` Laszlo Ersek
2018-08-02  1:24   ` Wu, Jiaxin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox