From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web11.5634.1624443331462256761 for ; Wed, 23 Jun 2021 03:15:31 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=OL8k4BXB; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1624443330; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=tlRU2m/9TJ/JaCH/KETq4dOaGZgWsJ7u2FAsGl2VPaI=; b=OL8k4BXBQZO3bpQ4Ki+IX2qkwy1nUmIT2nOyqUwpxUvq9wYNY1OGsuUx68VEzj1NuMI2DS PGSqmBOpvSZGHl3zoN63MABSuh+IArkul9zxQA9TEq5qZ/7KlreC56UP8OS0vOAwZ00bj8 4oRf+U48KXbWo6lu42ytuadvFSmroC0= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-116-nUYCc2MXOFagWdumDD9_eQ-1; Wed, 23 Jun 2021 06:15:27 -0400 X-MC-Unique: nUYCc2MXOFagWdumDD9_eQ-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 0664CA40C1; Wed, 23 Jun 2021 10:15:26 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-200.ams2.redhat.com [10.36.112.200]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 542681346F; Wed, 23 Jun 2021 10:15:24 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH] NetworkPkg: Addressed static code analyzer issues To: devel@edk2.groups.io, emergingsiva@gmail.com Cc: Sivaraman , Maciej Rabeda , Jiaxin Wu , Siyuan Fu References: From: "Laszlo Ersek" Message-ID: <59d83dd2-1cc8-9cf6-dfaa-3f2e0028ee35@redhat.com> Date: Wed, 23 Jun 2021 12:15:22 +0200 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 > --- > 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; >