On 08/06/18 09:49, shenglei wrote:
> Some dead code has been removed in .c and .h files.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1062
>
> Cc: Star Zeng <[email protected]>
> Cc: Eric Dong <[email protected]>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: shenglei <[email protected]>
> ---
> .../Application/CapsuleApp/CapsuleDump.c | 31 ---
> MdeModulePkg/Application/UiApp/FrontPage.c | 40 ---
> MdeModulePkg/Application/UiApp/Ui.h | 30 --
> .../Bus/Ata/AtaAtapiPassThru/AhciMode.c | 104 -------
> .../Bus/Ata/AtaAtapiPassThru/IdeMode.c | 257 ------------------
> .../Bus/Isa/Ps2KeyboardDxe/Ps2KbdCtrller.c | 26 --
> .../Bus/Isa/Ps2KeyboardDxe/Ps2Keyboard.h | 12 -
> MdeModulePkg/Bus/Pci/EhciDxe/EhciDebug.c | 27 --
> MdeModulePkg/Bus/Pci/EhciDxe/EhciDebug.h | 11 -
> MdeModulePkg/Bus/Pci/EhciDxe/EhciReg.c | 44 ---
> .../Bus/Pci/NvmExpressDxe/NvmExpressHci.c | 110 --------
> .../Bus/Pci/PciBusDxe/PciDeviceSupport.c | 80 ------
> .../Bus/Pci/PciBusDxe/PciDeviceSupport.h | 17 --
> MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 41 ---
> MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.h | 21 --
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 94 -------
> MdeModulePkg/Bus/Pci/UhciPei/DmaMem.c | 22 --
> MdeModulePkg/Bus/Pci/UhciPei/UhcPeim.c | 125 ---------
> MdeModulePkg/Bus/Pci/UhciPei/UhcPeim.h | 78 ------
> MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 66 -----
> MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h | 28 --
> MdeModulePkg/Bus/Pci/XhciPei/UsbHcMem.c | 24 --
> MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.c | 22 --
> MdeModulePkg/Bus/Pci/XhciPei/XhciReg.h | 14 -
> .../Bus/Sd/EmmcBlockIoPei/EmmcHcMem.c | 24 --
> MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHcMem.c | 24 --
> .../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.c | 101 -------
> MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHcMem.c | 24 --
> MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsHci.c | 180 ------------
> .../Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 49 ----
> MdeModulePkg/Bus/Usb/UsbBotPei/PeiUsbLib.c | 190 -------------
> MdeModulePkg/Bus/Usb/UsbBotPei/PeiUsbLib.h | 98 -------
> MdeModulePkg/Bus/Usb/UsbBusDxe/UsbHub.c | 68 -----
> MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.c | 146 ----------
> MdeModulePkg/Bus/Usb/UsbBusDxe/UsbUtility.h | 114 --------
> MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.c | 39 ---
> MdeModulePkg/Bus/Usb/UsbBusPei/HubPeim.h | 18 --
> MdeModulePkg/Bus/Usb/UsbBusPei/PeiUsbLib.c | 77 ------
> MdeModulePkg/Bus/Usb/UsbBusPei/PeiUsbLib.h | 35 ---
> MdeModulePkg/Core/Dxe/DxeMain.h | 13 -
> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 23 --
> MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 78 ------
> MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 166 -----------
> .../Core/PiSmmCore/MemoryAttributesTable.c | 131 ---------
> MdeModulePkg/Core/PiSmmCore/Page.c | 121 ---------
> .../Universal/Console/TerminalDxe/Terminal.h | 12 -
> .../Console/TerminalDxe/TerminalConIn.c | 25 --
> .../Universal/HiiDatabaseDxe/ConfigRouting.c | 47 ----
> .../Universal/Network/IScsiDxe/IScsiProto.c | 31 ---
> .../Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 16 --
> .../Universal/Network/Tcp4Dxe/SockImpl.c | 35 ---
> .../Universal/Network/Tcp4Dxe/SockInterface.c | 41 ---
> .../Universal/Network/Tcp4Dxe/Socket.h | 32 ---
> .../Universal/Network/Tcp4Dxe/Tcp4Option.c | 28 --
> .../Universal/Network/Tcp4Dxe/Tcp4Option.h | 15 -
> .../Universal/SetupBrowserDxe/IfrParse.c | 33 ---
> 56 files changed, 3358 deletions(-)
Please split this patch up to a series so that *at the least* each
module is covered by a separate patch. For example,
1 MdeModulePkg/Application/CapsuleApp
2 MdeModulePkg/Application/UiApp
3 MdeModulePkg/Bus/Ata/AtaAtapiPassThru
4 MdeModulePkg/Bus/Isa/Ps2KeyboardDxe
5 MdeModulePkg/Bus/Pci/EhciDxe
6 MdeModulePkg/Bus/Pci/NvmExpressDxe
7 MdeModulePkg/Bus/Pci/PciBusDxe
8 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe
9 MdeModulePkg/Bus/Pci/UhciPei
10 MdeModulePkg/Bus/Pci/XhciDxe
11 MdeModulePkg/Bus/Pci/XhciPei
12 MdeModulePkg/Bus/Sd/EmmcBlockIoPei
13 MdeModulePkg/Bus/Sd/SdBlockIoPei
14 MdeModulePkg/Bus/Ufs/UfsBlockIoPei
15 MdeModulePkg/Bus/Ufs/UfsPassThruDxe
16 MdeModulePkg/Bus/Usb/UsbBotPei
17 MdeModulePkg/Bus/Usb/UsbBusDxe
18 MdeModulePkg/Bus/Usb/UsbBusPei
19 MdeModulePkg/Core/Dxe
20 MdeModulePkg/Core/PiSmmCore
21 MdeModulePkg/Universal/Console/TerminalDxe
22 MdeModulePkg/Universal/HiiDatabaseDxe
23 MdeModulePkg/Universal/Network/IScsiDxe
24 MdeModulePkg/Universal/Network/Ip4Dxe
25 MdeModulePkg/Universal/Network/Tcp4Dxe
26 MdeModulePkg/Universal/SetupBrowserDxe
Removing 3358 lines, under a blanket "dead code" comment, across 26
drivers, is terribly bad practice, in my opinion. It's unreviewable, and
also unbisectable if something goes wrong.
The bug report <https://bugzilla.tianocore.org/show_bug.cgi?id=1062> states,
Uefi-aware compiler found many modules have dead code and redundant
definitions in the MdeModulePkg. Please see the detail info in the
attachment log. We need to clean them.
Because the module number is big, I report them all together by
package in this tracker and don't by modules separately.
Please be aware that there might be false positives in the log. The
Uefi-aware compiler currently only analyze the C code and has not
supported the assemble code yet. So, if a function is only invoked
by assemble code, the Uefi-aware compiler might miss it and report
it as dead code. Please verify them carefully if the module have
assemble code.
The bug report itself states that there might be false positives in the
log and that at least some cases have to be evaluated carefully. It's
exactly that evaluation that should be captured in each commit message.
The fact that the module number is big is *exactly* the reason why the
patch series should be fine-grained.
Many of these modules are consumed by most platforms in existence. If a
platform owner would like to take a look at a small subset of the
modules being modified here, they now have to dig around in a 3+ KLOC
patch, and figure it out for themselves *why* some code is called "dead".
Star, Eric, what are your takes on this? Do you agree with my request
above, or do you prefer the patch in its current form?
Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
