Hi Lazlo:
case Dhcp4SendRequest:
if (Packet->Length > PXEBC_DHCP4_PACKET_MAX_SIZE) {
//
// If the to be sent packet exceeds the maximum length, abort the DHCP
process.
//
Status = EFI_ABORTED;
break;
}
The same packet Length check already taken care on case Dhcp4SendDiscover: also.
So I prefer to remove the above set of lines.
Wil update the subject when sending V2 changes.
-Siva
-----Original Message-----
From: Laszlo Ersek <ler...@redhat.com>
Sent: Wednesday, June 23, 2021 3:45 PM
To: devel@edk2.groups.io; emergings...@gmail.com
Cc: Sivaraman Nainar <sivaram...@ami.com>; Maciej Rabeda
<maciej.rab...@linux.intel.com>; Jiaxin Wu <jiaxin...@intel.com>; Siyuan Fu
<siyuan...@intel.com>
Subject: [EXTERNAL] Re: [edk2-devel] [PATCH] NetworkPkg: Addressed static code
analyzer issues
**CAUTION: The e-mail below is from an external source. Please exercise caution
before opening attachments, clicking links, or following guidance.**
adding NetworkPkg maintainers, comments below
On 06/18/21 05:30, INDIA\sivaramann wrote:
> Issue on the PxeBcDhcp4CallBack() functions of UEFIPXEBC Driver.
> In this function allowed events are Dhcp4RcvdOffer, Dhcp4SelectOffer,
> Dhcp4SendDiscover, Dhcp4RcvdAck. If any other event comes as input it
> will exit in beginning itself.
Yes.
>
> Later below switch case handling the default case which is not reachable.
> I assume this code is a not reachable code and can be removed
(1) The edk2 coding style recommends adding "default" cases to switch
statements, as far as I recall. I'd keep the default, but add
ASSERT (FALSE);
there.
(2) There is a more confusing style issue with the same switch statement.
Namely, it has a case label for "Dhcp4SendRequest". Control will never jump to
that label, due to the "if" at the top of the function that you highlight.
Importantly, the *code* starting at the "Dhcp4SendRequest" case label must not
be removed, as the "Dhcp4SendDiscover" logic *falls through* to it. However,
the "Dhcp4SendRequest" case label itself should be removed, and the comment
just above it should be updated.
This dead label seems to originate from historical commit a3bcde70e6dc ("Add
NetworkPkg (P.UDK2010.UP3.Network.P1)", 2010-11-01).
(3) The subject line is nearly useless, please name at least
"NetworkPkg/UefiPxeBcDxe".
Thanks
Laszlo
>
> Signed-off-by: Sivaraman <sivaram...@ami.com>
> ---
> NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c
> b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c
> index fb63cf61a9..c0d8211ea0 100644
> --- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c
> +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c
> @@ -1331,8 +1331,6 @@ PxeBcDhcp4CallBack (
> }
> break;
>
> - default:
> - break;
> }
>
> return Status;
>
-The information contained in this message may be confidential and proprietary
to American Megatrends (AMI). This communication is intended to be read only by
the individual or entity to whom it is addressed or by their designee. If the
reader of this message is not the intended recipient, you are on notice that
any distribution of this message, in any form, is strictly prohibited. Please
promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and
then delete or destroy all copies of the transmission.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76966): https://edk2.groups.io/g/devel/message/76966
Mute This Topic: https://groups.io/mt/83735967/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
