From: "Fu, Siyuan" <siyuan.fu@intel.com>
To: "Wang, Fan" <fan.wang@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Ye, Ting" <ting.ye@intel.com>, "Wang, Fan" <fan.wang@intel.com>,
"Wu, Jiaxin" <jiaxin.wu@intel.com>
Subject: Re: [Patch] MdeModulePkg/DxeUdpIoLib: Did some code enhancement for UdpIoLib
Date: Thu, 4 Jan 2018 06:15:00 +0000 [thread overview]
Message-ID: <B1FF2E9001CE9041BD10B825821D5BC58B40CF14@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <1515045735-9004-1-git-send-email-fan.wang@intel.com>
Hi, Fan
+ If Udp version is not Udp4 or Udp6, then ASSERT().
It's better to update "Udp4" and "Udp6" to " UDP_IO_UDP4_VERSION" and " UDP_IO_UDP6_VERSION"
Other parts are good with me.
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> fanwang2
> Sent: Thursday, January 4, 2018 2:02 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Wang, Fan <fan.wang@intel.com>; Fu,
> Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [edk2] [Patch] MdeModulePkg/DxeUdpIoLib: Did some code
> enhancement for UdpIoLib
>
> From: Wang Fan <fan.wang@intel.com>
>
> * Added some ASSERT descriptions for library APIs.
> * Added "Optional" option for Context parameter in UdpIoCancelDgrams().
> * Added function return status check for UdpIoFreeIo().
>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan <fan.wang@intel.com>
> ---
> MdeModulePkg/Include/Library/UdpIoLib.h | 16 ++++-
> MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c | 90 ++++++++++++++++++---
> -----
> 2 files changed, 78 insertions(+), 28 deletions(-)
>
> diff --git a/MdeModulePkg/Include/Library/UdpIoLib.h
> b/MdeModulePkg/Include/Library/UdpIoLib.h
> index e0b44ce..86a11a9 100644
> --- a/MdeModulePkg/Include/Library/UdpIoLib.h
> +++ b/MdeModulePkg/Include/Library/UdpIoLib.h
> @@ -195,11 +195,13 @@ BOOLEAN
> IN VOID *Context
> );
>
> /**
> Cancel all the sent datagram that pass the selection criteria of
> ToCancel.
> +
> If ToCancel is NULL, all the datagrams are cancelled.
> + If Udp version is not Udp4 or Udp6, then ASSERT().
>
> @param[in] UdpIo The UDP_IO to cancel packet.
> @param[in] IoStatus The IoStatus to return to the packet
> owners.
> @param[in] ToCancel The select funtion to test whether to
> cancel this
> packet or not.
> @@ -210,17 +212,20 @@ VOID
> EFIAPI
> UdpIoCancelDgrams (
> IN UDP_IO *UdpIo,
> IN EFI_STATUS IoStatus,
> IN UDP_IO_TO_CANCEL ToCancel, OPTIONAL
> - IN VOID *Context
> + IN VOID *Context OPTIONAL
> );
>
> /**
> Creates a UDP_IO to access the UDP service. It creates and configures
> a UDP child.
>
> + If Configure is NULL, then ASSERT().
> + If Udp version is not Udp4 or Udp6, then ASSERT().
> +
> It locates the UDP service binding prototype on the Controller
> parameter
> uses the UDP service binding prototype to create a UDP child (also
> known as
> a UDP instance) configures the UDP child by calling Configure function
> prototype.
> Any failures in creating or configuring the UDP child return NULL for
> failure.
>
> @@ -245,15 +250,18 @@ UdpIoCreateIo (
> );
>
> /**
> Free the UDP_IO and all its related resources.
>
> + If Udp version is not Udp4 or Udp6, then ASSERT().
> +
> The function cancels all sent datagrams and receive requests.
>
> @param[in] UdpIo The UDP_IO to free.
>
> @retval EFI_SUCCESS The UDP_IO is freed.
> + @retval Others Failed to free UDP_IO.
>
> **/
> EFI_STATUS
> EFIAPI
> UdpIoFreeIo (
> @@ -262,10 +270,12 @@ UdpIoFreeIo (
>
> /**
> Cleans up the UDP_IO without freeing it. Call this function
> if you intend to later re-use the UDP_IO.
>
> + If Udp version is not Udp4 or Udp6, then ASSERT().
> +
> This function releases all transmitted datagrams and receive requests
> and configures NULL for the UDP instance.
>
> @param[in] UdpIo The UDP_IO to clean up.
>
> **/
> @@ -276,10 +286,12 @@ UdpIoCleanIo (
> );
>
> /**
> Send a packet through the UDP_IO.
>
> + If Udp version is not Udp4 or Udp6, then ASSERT().
> +
> The packet will be wrapped in UDP_TX_TOKEN. Function Callback will be
> called
> when the packet is sent. The optional parameter EndPoint overrides the
> default
> address pair if specified.
>
> @param[in] UdpIo The UDP_IO to send the packet through.
> @@ -322,10 +334,12 @@ UdpIoCancelSentDatagram (
> );
>
> /**
> Issue a receive request to the UDP_IO.
>
> + If Udp version is not Udp4 or Udp6, then ASSERT().
> +
> This function is called when upper-layer needs packet from UDP for
> processing.
> Only one receive request is acceptable at a time. Therefore, one common
> usage model is
> to invoke this function inside its Callback function when the former
> packet
> is processed.
>
> diff --git a/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c
> b/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c
> index 4861095..e2d3466 100644
> --- a/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c
> +++ b/MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.c
> @@ -408,10 +408,13 @@ UdpIoCreateRxToken (
> }
>
> /**
> Wrap a transmit request into a new created UDP_TX_TOKEN.
>
> + If Packet is NULL, then ASSERT().
> + If Udp version is not Udp4 or Udp6, then ASSERT().
> +
> @param[in] UdpIo The UdpIo to send packet to.
> @param[in] Packet The user's packet.
> @param[in] EndPoint The local and remote access point.
> @param[in] Gateway The overrided next hop.
> @param[in] CallBack The function to call when
> transmission completed.
> @@ -578,10 +581,13 @@ UdpIoCreateTxToken (
>
> /**
> Creates a UDP_IO to access the UDP service. It creates and configures
> a UDP child.
>
> + If Configure is NULL, then ASSERT().
> + If Udp version is not Udp4 or Udp6, then ASSERT().
> +
> It locates the UDP service binding prototype on the Controller
> parameter
> uses the UDP service binding prototype to create a UDP child (also
> known as
> a UDP instance) configures the UDP child by calling Configure function
> prototype.
> Any failures in creating or configuring the UDP child return NULL for
> failure.
>
> @@ -747,11 +753,13 @@ FREE_MEM:
> return NULL;
> }
>
> /**
> Cancel all the sent datagram that pass the selection criteria of
> ToCancel.
> +
> If ToCancel is NULL, all the datagrams are cancelled.
> + If Udp version is not Udp4 or Udp6, then ASSERT().
>
> @param[in] UdpIo The UDP_IO to cancel packet.
> @param[in] IoStatus The IoStatus to return to the packet
> owners.
> @param[in] ToCancel The select funtion to test whether to
> cancel this
> packet or not.
> @@ -762,11 +770,11 @@ VOID
> EFIAPI
> UdpIoCancelDgrams (
> IN UDP_IO *UdpIo,
> IN EFI_STATUS IoStatus,
> IN UDP_IO_TO_CANCEL ToCancel, OPTIONAL
> - IN VOID *Context
> + IN VOID *Context OPTIONAL
> )
> {
> LIST_ENTRY *Entry;
> LIST_ENTRY *Next;
> UDP_TX_TOKEN *TxToken;
> @@ -789,23 +797,27 @@ UdpIoCancelDgrams (
> }
>
> /**
> Free the UDP_IO and all its related resources.
>
> + If Udp version is not Udp4 or Udp6, then ASSERT().
> +
> The function will cancel all sent datagram and receive request.
>
> @param[in] UdpIo The UDP_IO to free.
>
> @retval EFI_SUCCESS The UDP_IO is freed.
> + @retval Others Failed to free UDP_IO.
>
> **/
> EFI_STATUS
> EFIAPI
> UdpIoFreeIo (
> IN UDP_IO *UdpIo
> )
> {
> + EFI_STATUS Status;
> UDP_RX_TOKEN *RxToken;
>
> ASSERT ((UdpIo->UdpVersion == UDP_IO_UDP4_VERSION) ||
> (UdpIo->UdpVersion == UDP_IO_UDP6_VERSION));
>
> @@ -820,53 +832,71 @@ UdpIoFreeIo (
> UdpIoCancelDgrams (UdpIo, EFI_ABORTED, NULL, NULL);
>
> if (UdpIo->UdpVersion == UDP_IO_UDP4_VERSION) {
>
> if ((RxToken = UdpIo->RecvRequest) != NULL) {
> - UdpIo->Protocol.Udp4->Cancel (UdpIo->Protocol.Udp4, &RxToken-
> >Token.Udp4);
> + Status = UdpIo->Protocol.Udp4->Cancel (UdpIo->Protocol.Udp4,
> &RxToken->Token.Udp4);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> }
>
> //
> // Close then destroy the Udp4 child
> //
> - gBS->CloseProtocol (
> - UdpIo->UdpHandle,
> - &gEfiUdp4ProtocolGuid,
> - UdpIo->Image,
> - UdpIo->Controller
> - );
> + Status = gBS->CloseProtocol (
> + UdpIo->UdpHandle,
> + &gEfiUdp4ProtocolGuid,
> + UdpIo->Image,
> + UdpIo->Controller
> + );
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
>
> - NetLibDestroyServiceChild (
> - UdpIo->Controller,
> - UdpIo->Image,
> - &gEfiUdp4ServiceBindingProtocolGuid,
> - UdpIo->UdpHandle
> - );
> + Status = NetLibDestroyServiceChild (
> + UdpIo->Controller,
> + UdpIo->Image,
> + &gEfiUdp4ServiceBindingProtocolGuid,
> + UdpIo->UdpHandle
> + );
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
>
> } else {
>
> if ((RxToken = UdpIo->RecvRequest) != NULL) {
> - UdpIo->Protocol.Udp6->Cancel (UdpIo->Protocol.Udp6, &RxToken-
> >Token.Udp6);
> + Status = UdpIo->Protocol.Udp6->Cancel (UdpIo->Protocol.Udp6,
> &RxToken->Token.Udp6);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> }
>
> //
> // Close then destroy the Udp6 child
> //
> - gBS->CloseProtocol (
> - UdpIo->UdpHandle,
> - &gEfiUdp6ProtocolGuid,
> - UdpIo->Image,
> - UdpIo->Controller
> - );
> + Status = gBS->CloseProtocol (
> + UdpIo->UdpHandle,
> + &gEfiUdp6ProtocolGuid,
> + UdpIo->Image,
> + UdpIo->Controller
> + );
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
>
> - NetLibDestroyServiceChild (
> - UdpIo->Controller,
> - UdpIo->Image,
> - &gEfiUdp6ServiceBindingProtocolGuid,
> - UdpIo->UdpHandle
> - );
> + Status = NetLibDestroyServiceChild (
> + UdpIo->Controller,
> + UdpIo->Image,
> + &gEfiUdp6ServiceBindingProtocolGuid,
> + UdpIo->UdpHandle
> + );
> + if (EFI_ERROR (Status)) {
> + return Status;
> }
> + }
>
> if (!IsListEmpty(&UdpIo->Link)) {
> RemoveEntryList (&UdpIo->Link);
> }
>
> @@ -877,10 +907,12 @@ UdpIoFreeIo (
>
> /**
> Clean up the UDP_IO without freeing it. The function is called when
> user wants to re-use the UDP_IO later.
>
> + If Udp version is not Udp4 or Udp6, then ASSERT().
> +
> It will release all the transmitted datagrams and receive request. It
> will
> also configure NULL for the UDP instance.
>
> @param[in] UdpIo The UDP_IO to clean up.
>
> @@ -918,10 +950,12 @@ UdpIoCleanIo (
> }
>
> /**
> Send a packet through the UDP_IO.
>
> + If Udp version is not Udp4 or Udp6, then ASSERT().
> +
> The packet will be wrapped in UDP_TX_TOKEN. Function Callback will be
> called
> when the packet is sent. The optional parameter EndPoint overrides the
> default
> address pair if specified.
>
> @param[in] UdpIo The UDP_IO to send the packet through.
> @@ -1028,10 +1062,12 @@ UdpIoCancelSentDatagram (
> }
>
> /**
> Issue a receive request to the UDP_IO.
>
> + If Udp version is not Udp4 or Udp6, then ASSERT().
> +
> This function is called when upper-layer needs packet from UDP for
> processing.
> Only one receive request is acceptable at a time so a common usage
> model is
> to invoke this function inside its Callback function when the former
> packet
> is processed.
>
> --
> 1.9.5.msysgit.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
prev parent reply other threads:[~2018-01-04 6:10 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-04 6:02 [Patch] MdeModulePkg/DxeUdpIoLib: Did some code enhancement for UdpIoLib fanwang2
2018-01-04 6:15 ` Fu, Siyuan [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=B1FF2E9001CE9041BD10B825821D5BC58B40CF14@SHSMSX103.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox