Laszlo: We manually search the unused functions in the module source files, and also verify the build to make sure the unused functions are real dead code. I agree to separate this patch as the module level. So, we can document which unused functions are removed in the driver.
Thanks Liming > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of Laszlo > Ersek > Sent: Monday, August 6, 2018 9:12 PM > To: Zhang, Shenglei <[email protected]>; [email protected] > Cc: Dong, Eric <[email protected]>; Zeng, Star <[email protected]> > Subject: Re: [edk2] [PATCH] MdeModulePkg: Remove dead code in .c and .h files > > 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 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
