public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [RFC PATCH] RedfishPkg: RedfishDiscoverDxe: add [] brackets to URI for IPv6 addresses
@ 2023-12-07 13:24 Mike Maslenkin
  2023-12-13 11:09 ` Chang, Abner via groups.io
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Mike Maslenkin @ 2023-12-07 13:24 UTC (permalink / raw)
  To: devel; +Cc: Abner Chang, Nickle Wang, Igor Kulchytskyy

URI is generated based on the RedfishLocation containing an ASCII string
representing the IP address. So, in the case of IPv4 the canonical
representation of an IPv4 address was inserted into the resulting Unicode
string i.e: "http{,s}://X.X.X.X/".

In the case of IPv6, to access resources, the IP address must be specified
in brackets, i.e. the resulting string should look like:
  "http{,s}://[X::X:X:X:X]/".

Cc: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 .../RedfishDiscoverDxe/RedfishDiscoverDxe.c   | 20 ++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
index 28ba2d3a9fca..49c96bd28b27 100644
--- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
+++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
@@ -863,9 +863,23 @@ AddAndSignalNewRedfishService (
     }
 
     if (RedfishLocation != NULL) {
-      DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool (AsciiStrSize ((const CHAR8 *)RedfishLocation) * sizeof (CHAR16));
-      AsciiStrToUnicodeStrS ((const CHAR8 *)RedfishLocation, DiscoveredInstance->Information.Location, AsciiStrSize ((const CHAR8 *)RedfishLocation) * sizeof (CHAR16));
-      DEBUG ((DEBUG_MANAGEABILITY, "Redfish service location: %s.\n", DiscoveredInstance->Information.Location));
+      UINTN        AllocSize;
+      CONST CHAR8  *IpAddress;
+
+      IpAddress = (CONST CHAR8 *)RedfishLocation;
+      AllocSize = AsciiStrSize (IpAddress) * sizeof (CHAR16);
+
+      if (CheckIsIpVersion6 (NetworkInterface)) {
+        AllocSize += 2 * sizeof (CHAR16); // take into account '[' and ']'
+
+        DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool (AllocSize);
+        UnicodeSPrintAsciiFormat (DiscoveredInstance->Information.Location, AllocSize, "[%a]", IpAddress);
+      } else {
+        DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool (AllocSize);
+        AsciiStrToUnicodeStrS (IpAddress, DiscoveredInstance->Information.Location, AllocSize);
+      }
+
+      DEBUG ((DEBUG_MANAGEABILITY, "Redfish service location: %s\n", DiscoveredInstance->Information.Location));
     }
 
     if (Uuid != NULL) {
-- 
2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112189): https://edk2.groups.io/g/devel/message/112189
Mute This Topic: https://groups.io/mt/103033764/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [RFC PATCH] RedfishPkg: RedfishDiscoverDxe: add [] brackets to URI for IPv6 addresses
  2023-12-07 13:24 [edk2-devel] [RFC PATCH] RedfishPkg: RedfishDiscoverDxe: add [] brackets to URI for IPv6 addresses Mike Maslenkin
@ 2023-12-13 11:09 ` Chang, Abner via groups.io
  2023-12-13 13:15 ` Pedro Falcato
  2023-12-13 13:28 ` Pedro Falcato
  2 siblings, 0 replies; 7+ messages in thread
From: Chang, Abner via groups.io @ 2023-12-13 11:09 UTC (permalink / raw)
  To: Mike Maslenkin, devel@edk2.groups.io; +Cc: Nickle Wang, Igor Kulchytskyy

[AMD Official Use Only - General]

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Thursday, December 7, 2023 9:24 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; Nickle Wang
> <nicklew@nvidia.com>; Igor Kulchytskyy <igork@ami.com>
> Subject: [RFC PATCH] RedfishPkg: RedfishDiscoverDxe: add [] brackets to URI
> for IPv6 addresses
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> URI is generated based on the RedfishLocation containing an ASCII string
> representing the IP address. So, in the case of IPv4 the canonical
> representation of an IPv4 address was inserted into the resulting Unicode
> string i.e: "http{,s}://X.X.X.X/".
>
> In the case of IPv6, to access resources, the IP address must be specified
> in brackets, i.e. the resulting string should look like:
>   "http{,s}://[X::X:X:X:X]/".
>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  .../RedfishDiscoverDxe/RedfishDiscoverDxe.c   | 20 ++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 28ba2d3a9fca..49c96bd28b27 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -863,9 +863,23 @@ AddAndSignalNewRedfishService (
>      }
>
>
>
>      if (RedfishLocation != NULL) {
>
> -      DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool
> (AsciiStrSize ((const CHAR8 *)RedfishLocation) * sizeof (CHAR16));
>
> -      AsciiStrToUnicodeStrS ((const CHAR8 *)RedfishLocation,
> DiscoveredInstance->Information.Location, AsciiStrSize ((const CHAR8
> *)RedfishLocation) * sizeof (CHAR16));
>
> -      DEBUG ((DEBUG_MANAGEABILITY, "Redfish service location: %s.\n",
> DiscoveredInstance->Information.Location));
>
> +      UINTN        AllocSize;
>
> +      CONST CHAR8  *IpAddress;
Please move the local variable declarations at the beginning of this function.
All others look good to me.
Thanks
Abner

>
> +
>
> +      IpAddress = (CONST CHAR8 *)RedfishLocation;
>
> +      AllocSize = AsciiStrSize (IpAddress) * sizeof (CHAR16);
>
> +
>
> +      if (CheckIsIpVersion6 (NetworkInterface)) {
>
> +        AllocSize += 2 * sizeof (CHAR16); // take into account '[' and ']'
>
> +
>
> +        DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool
> (AllocSize);
>
> +        UnicodeSPrintAsciiFormat (DiscoveredInstance->Information.Location,
> AllocSize, "[%a]", IpAddress);
>
> +      } else {
>
> +        DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool
> (AllocSize);
>
> +        AsciiStrToUnicodeStrS (IpAddress, DiscoveredInstance-
> >Information.Location, AllocSize);
>
> +      }
>
> +
>
> +      DEBUG ((DEBUG_MANAGEABILITY, "Redfish service location: %s\n",
> DiscoveredInstance->Information.Location));
>
>      }
>
>
>
>      if (Uuid != NULL) {
>
> --
> 2.32.0 (Apple Git-132)



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112470): https://edk2.groups.io/g/devel/message/112470
Mute This Topic: https://groups.io/mt/103033764/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [RFC PATCH] RedfishPkg: RedfishDiscoverDxe: add [] brackets to URI for IPv6 addresses
  2023-12-07 13:24 [edk2-devel] [RFC PATCH] RedfishPkg: RedfishDiscoverDxe: add [] brackets to URI for IPv6 addresses Mike Maslenkin
  2023-12-13 11:09 ` Chang, Abner via groups.io
@ 2023-12-13 13:15 ` Pedro Falcato
  2023-12-13 13:35   ` Mike Maslenkin
  2023-12-15  0:17   ` Chang, Abner via groups.io
  2023-12-13 13:28 ` Pedro Falcato
  2 siblings, 2 replies; 7+ messages in thread
From: Pedro Falcato @ 2023-12-13 13:15 UTC (permalink / raw)
  To: devel, mike.maslenkin; +Cc: Abner Chang, Nickle Wang, Igor Kulchytskyy

On Thu, Dec 7, 2023 at 1:24 PM Mike Maslenkin <mike.maslenkin@gmail.com> wrote:
>
> URI is generated based on the RedfishLocation containing an ASCII string
> representing the IP address. So, in the case of IPv4 the canonical
> representation of an IPv4 address was inserted into the resulting Unicode
> string i.e: "http{,s}://X.X.X.X/".
>
> In the case of IPv6, to access resources, the IP address must be specified
> in brackets, i.e. the resulting string should look like:
>   "http{,s}://[X::X:X:X:X]/".
>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  .../RedfishDiscoverDxe/RedfishDiscoverDxe.c   | 20 ++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 28ba2d3a9fca..49c96bd28b27 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -863,9 +863,23 @@ AddAndSignalNewRedfishService (
>      }
>
>      if (RedfishLocation != NULL) {
> -      DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool (AsciiStrSize ((const CHAR8 *)RedfishLocation) * sizeof (CHAR16));
> -      AsciiStrToUnicodeStrS ((const CHAR8 *)RedfishLocation, DiscoveredInstance->Information.Location, AsciiStrSize ((const CHAR8 *)RedfishLocation) * sizeof (CHAR16));
> -      DEBUG ((DEBUG_MANAGEABILITY, "Redfish service location: %s.\n", DiscoveredInstance->Information.Location));
> +      UINTN        AllocSize;
> +      CONST CHAR8  *IpAddress;
> +
> +      IpAddress = (CONST CHAR8 *)RedfishLocation;
> +      AllocSize = AsciiStrSize (IpAddress) * sizeof (CHAR16);
> +
> +      if (CheckIsIpVersion6 (NetworkInterface)) {
> +        AllocSize += 2 * sizeof (CHAR16); // take into account '[' and ']'
> +
> +        DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool (AllocSize);

You don't check for NULL.

> +        UnicodeSPrintAsciiFormat (DiscoveredInstance->Information.Location, AllocSize, "[%a]", IpAddress);
> +      } else {
> +        DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool (AllocSize);

You don't check for NULL.
Heck, why does no one check for NULL in this whole function
(AddAndSignalNewRedfishService)?

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112472): https://edk2.groups.io/g/devel/message/112472
Mute This Topic: https://groups.io/mt/103033764/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [RFC PATCH] RedfishPkg: RedfishDiscoverDxe: add [] brackets to URI for IPv6 addresses
  2023-12-07 13:24 [edk2-devel] [RFC PATCH] RedfishPkg: RedfishDiscoverDxe: add [] brackets to URI for IPv6 addresses Mike Maslenkin
  2023-12-13 11:09 ` Chang, Abner via groups.io
  2023-12-13 13:15 ` Pedro Falcato
@ 2023-12-13 13:28 ` Pedro Falcato
  2 siblings, 0 replies; 7+ messages in thread
From: Pedro Falcato @ 2023-12-13 13:28 UTC (permalink / raw)
  To: devel, mike.maslenkin; +Cc: Abner Chang, Nickle Wang, Igor Kulchytskyy

On Thu, Dec 7, 2023 at 1:24 PM Mike Maslenkin <mike.maslenkin@gmail.com> wrote:
> [...]
> +        DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool (AllocSize);

Forgot to mention: don't cast void *! It's completely unneeded in C
and may hide real problems (implicit declarations); it also clutters
code.

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112473): https://edk2.groups.io/g/devel/message/112473
Mute This Topic: https://groups.io/mt/103033764/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [RFC PATCH] RedfishPkg: RedfishDiscoverDxe: add [] brackets to URI for IPv6 addresses
  2023-12-13 13:15 ` Pedro Falcato
@ 2023-12-13 13:35   ` Mike Maslenkin
  2023-12-13 13:38     ` Chang, Abner via groups.io
  2023-12-15  0:17   ` Chang, Abner via groups.io
  1 sibling, 1 reply; 7+ messages in thread
From: Mike Maslenkin @ 2023-12-13 13:35 UTC (permalink / raw)
  To: Pedro Falcato; +Cc: devel, Abner Chang, Nickle Wang, Igor Kulchytskyy

Fair.
I'll try to make some cleanups before applying this patch after
committing Abner's remedy patch
https://github.com/tianocore/edk2/pull/5139.

Regards,
Mike..


On Wed, Dec 13, 2023 at 4:15 PM Pedro Falcato <pedro.falcato@gmail.com> wrote:
>
> On Thu, Dec 7, 2023 at 1:24 PM Mike Maslenkin <mike.maslenkin@gmail.com> wrote:
> >
> > URI is generated based on the RedfishLocation containing an ASCII string
> > representing the IP address. So, in the case of IPv4 the canonical
> > representation of an IPv4 address was inserted into the resulting Unicode
> > string i.e: "http{,s}://X.X.X.X/".
> >
> > In the case of IPv6, to access resources, the IP address must be specified
> > in brackets, i.e. the resulting string should look like:
> >   "http{,s}://[X::X:X:X:X]/".
> >
> > Cc: Abner Chang <abner.chang@amd.com>
> > Cc: Nickle Wang <nicklew@nvidia.com>
> > Cc: Igor Kulchytskyy <igork@ami.com>
> > Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> > ---
> >  .../RedfishDiscoverDxe/RedfishDiscoverDxe.c   | 20 ++++++++++++++++---
> >  1 file changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > index 28ba2d3a9fca..49c96bd28b27 100644
> > --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > @@ -863,9 +863,23 @@ AddAndSignalNewRedfishService (
> >      }
> >
> >      if (RedfishLocation != NULL) {
> > -      DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool (AsciiStrSize ((const CHAR8 *)RedfishLocation) * sizeof (CHAR16));
> > -      AsciiStrToUnicodeStrS ((const CHAR8 *)RedfishLocation, DiscoveredInstance->Information.Location, AsciiStrSize ((const CHAR8 *)RedfishLocation) * sizeof (CHAR16));
> > -      DEBUG ((DEBUG_MANAGEABILITY, "Redfish service location: %s.\n", DiscoveredInstance->Information.Location));
> > +      UINTN        AllocSize;
> > +      CONST CHAR8  *IpAddress;
> > +
> > +      IpAddress = (CONST CHAR8 *)RedfishLocation;
> > +      AllocSize = AsciiStrSize (IpAddress) * sizeof (CHAR16);
> > +
> > +      if (CheckIsIpVersion6 (NetworkInterface)) {
> > +        AllocSize += 2 * sizeof (CHAR16); // take into account '[' and ']'
> > +
> > +        DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool (AllocSize);
>
> You don't check for NULL.
>
> > +        UnicodeSPrintAsciiFormat (DiscoveredInstance->Information.Location, AllocSize, "[%a]", IpAddress);
> > +      } else {
> > +        DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool (AllocSize);
>
> You don't check for NULL.
> Heck, why does no one check for NULL in this whole function
> (AddAndSignalNewRedfishService)?
>
> --
> Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112474): https://edk2.groups.io/g/devel/message/112474
Mute This Topic: https://groups.io/mt/103033764/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [RFC PATCH] RedfishPkg: RedfishDiscoverDxe: add [] brackets to URI for IPv6 addresses
  2023-12-13 13:35   ` Mike Maslenkin
@ 2023-12-13 13:38     ` Chang, Abner via groups.io
  0 siblings, 0 replies; 7+ messages in thread
From: Chang, Abner via groups.io @ 2023-12-13 13:38 UTC (permalink / raw)
  To: Mike Maslenkin, Pedro Falcato
  Cc: devel@edk2.groups.io, Nickle Wang, Igor Kulchytskyy

[AMD Official Use Only - General]

And Mike, you can also go head to create a PR after you sending out the patch. Thus I can merge it later.

Thanks
Abner

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Wednesday, December 13, 2023 9:35 PM
> To: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>; Nickle
> Wang <nicklew@nvidia.com>; Igor Kulchytskyy <igork@ami.com>
> Subject: Re: [edk2-devel] [RFC PATCH] RedfishPkg: RedfishDiscoverDxe: add []
> brackets to URI for IPv6 addresses
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Fair.
> I'll try to make some cleanups before applying this patch after
> committing Abner's remedy patch
> https://github.com/tianocore/edk2/pull/5139.
>
> Regards,
> Mike..
>
>
> On Wed, Dec 13, 2023 at 4:15 PM Pedro Falcato <pedro.falcato@gmail.com>
> wrote:
> >
> > On Thu, Dec 7, 2023 at 1:24 PM Mike Maslenkin
> <mike.maslenkin@gmail.com> wrote:
> > >
> > > URI is generated based on the RedfishLocation containing an ASCII string
> > > representing the IP address. So, in the case of IPv4 the canonical
> > > representation of an IPv4 address was inserted into the resulting Unicode
> > > string i.e: "http{,s}://X.X.X.X/".
> > >
> > > In the case of IPv6, to access resources, the IP address must be specified
> > > in brackets, i.e. the resulting string should look like:
> > >   "http{,s}://[X::X:X:X:X]/".
> > >
> > > Cc: Abner Chang <abner.chang@amd.com>
> > > Cc: Nickle Wang <nicklew@nvidia.com>
> > > Cc: Igor Kulchytskyy <igork@ami.com>
> > > Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> > > ---
> > >  .../RedfishDiscoverDxe/RedfishDiscoverDxe.c   | 20 ++++++++++++++++--
> -
> > >  1 file changed, 17 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > > index 28ba2d3a9fca..49c96bd28b27 100644
> > > --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > > +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > > @@ -863,9 +863,23 @@ AddAndSignalNewRedfishService (
> > >      }
> > >
> > >      if (RedfishLocation != NULL) {
> > > -      DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool
> (AsciiStrSize ((const CHAR8 *)RedfishLocation) * sizeof (CHAR16));
> > > -      AsciiStrToUnicodeStrS ((const CHAR8 *)RedfishLocation,
> DiscoveredInstance->Information.Location, AsciiStrSize ((const CHAR8
> *)RedfishLocation) * sizeof (CHAR16));
> > > -      DEBUG ((DEBUG_MANAGEABILITY, "Redfish service location: %s.\n",
> DiscoveredInstance->Information.Location));
> > > +      UINTN        AllocSize;
> > > +      CONST CHAR8  *IpAddress;
> > > +
> > > +      IpAddress = (CONST CHAR8 *)RedfishLocation;
> > > +      AllocSize = AsciiStrSize (IpAddress) * sizeof (CHAR16);
> > > +
> > > +      if (CheckIsIpVersion6 (NetworkInterface)) {
> > > +        AllocSize += 2 * sizeof (CHAR16); // take into account '[' and ']'
> > > +
> > > +        DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool
> (AllocSize);
> >
> > You don't check for NULL.
> >
> > > +        UnicodeSPrintAsciiFormat (DiscoveredInstance-
> >Information.Location, AllocSize, "[%a]", IpAddress);
> > > +      } else {
> > > +        DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool
> (AllocSize);
> >
> > You don't check for NULL.
> > Heck, why does no one check for NULL in this whole function
> > (AddAndSignalNewRedfishService)?
> >
> > --
> > Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112475): https://edk2.groups.io/g/devel/message/112475
Mute This Topic: https://groups.io/mt/103033764/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [RFC PATCH] RedfishPkg: RedfishDiscoverDxe: add [] brackets to URI for IPv6 addresses
  2023-12-13 13:15 ` Pedro Falcato
  2023-12-13 13:35   ` Mike Maslenkin
@ 2023-12-15  0:17   ` Chang, Abner via groups.io
  1 sibling, 0 replies; 7+ messages in thread
From: Chang, Abner via groups.io @ 2023-12-15  0:17 UTC (permalink / raw)
  To: Pedro Falcato, devel@edk2.groups.io, mike.maslenkin@gmail.com
  Cc: Nickle Wang, Igor Kulchytskyy

[AMD Official Use Only - General]

> -----Original Message-----
> From: Pedro Falcato <pedro.falcato@gmail.com>
> Sent: Wednesday, December 13, 2023 9:15 PM
> To: devel@edk2.groups.io; mike.maslenkin@gmail.com
> Cc: Chang, Abner <Abner.Chang@amd.com>; Nickle Wang
> <nicklew@nvidia.com>; Igor Kulchytskyy <igork@ami.com>
> Subject: Re: [edk2-devel] [RFC PATCH] RedfishPkg: RedfishDiscoverDxe: add []
> brackets to URI for IPv6 addresses
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Thu, Dec 7, 2023 at 1:24 PM Mike Maslenkin
> <mike.maslenkin@gmail.com> wrote:
> >
> > URI is generated based on the RedfishLocation containing an ASCII string
> > representing the IP address. So, in the case of IPv4 the canonical
> > representation of an IPv4 address was inserted into the resulting Unicode
> > string i.e: "http{,s}://X.X.X.X/".
> >
> > In the case of IPv6, to access resources, the IP address must be specified
> > in brackets, i.e. the resulting string should look like:
> >   "http{,s}://[X::X:X:X:X]/".
> >
> > Cc: Abner Chang <abner.chang@amd.com>
> > Cc: Nickle Wang <nicklew@nvidia.com>
> > Cc: Igor Kulchytskyy <igork@ami.com>
> > Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
> > ---
> >  .../RedfishDiscoverDxe/RedfishDiscoverDxe.c   | 20 ++++++++++++++++---
> >  1 file changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > index 28ba2d3a9fca..49c96bd28b27 100644
> > --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > @@ -863,9 +863,23 @@ AddAndSignalNewRedfishService (
> >      }
> >
> >      if (RedfishLocation != NULL) {
> > -      DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool
> (AsciiStrSize ((const CHAR8 *)RedfishLocation) * sizeof (CHAR16));
> > -      AsciiStrToUnicodeStrS ((const CHAR8 *)RedfishLocation,
> DiscoveredInstance->Information.Location, AsciiStrSize ((const CHAR8
> *)RedfishLocation) * sizeof (CHAR16));
> > -      DEBUG ((DEBUG_MANAGEABILITY, "Redfish service location: %s.\n",
> DiscoveredInstance->Information.Location));
> > +      UINTN        AllocSize;
> > +      CONST CHAR8  *IpAddress;
> > +
> > +      IpAddress = (CONST CHAR8 *)RedfishLocation;
> > +      AllocSize = AsciiStrSize (IpAddress) * sizeof (CHAR16);
> > +
> > +      if (CheckIsIpVersion6 (NetworkInterface)) {
> > +        AllocSize += 2 * sizeof (CHAR16); // take into account '[' and ']'
> > +
> > +        DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool
> (AllocSize);
>
> You don't check for NULL.
>
> > +        UnicodeSPrintAsciiFormat (DiscoveredInstance->Information.Location,
> AllocSize, "[%a]", IpAddress);
> > +      } else {
> > +        DiscoveredInstance->Information.Location = (CHAR16 *)AllocatePool
> (AllocSize);
>
> You don't check for NULL.
> Heck, why does no one check for NULL in this whole function
> (AddAndSignalNewRedfishService)?
Yes, we should check it. Could you please create a Bugzilla ticket for this?

Thanks
Abner

>
> --
> Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112578): https://edk2.groups.io/g/devel/message/112578
Mute This Topic: https://groups.io/mt/103033764/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

end of thread, other threads:[~2023-12-15  0:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-07 13:24 [edk2-devel] [RFC PATCH] RedfishPkg: RedfishDiscoverDxe: add [] brackets to URI for IPv6 addresses Mike Maslenkin
2023-12-13 11:09 ` Chang, Abner via groups.io
2023-12-13 13:15 ` Pedro Falcato
2023-12-13 13:35   ` Mike Maslenkin
2023-12-13 13:38     ` Chang, Abner via groups.io
2023-12-15  0:17   ` Chang, Abner via groups.io
2023-12-13 13:28 ` Pedro Falcato

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