On 7/19/23 10:19 PM, Ni, Ray wrote:
Taylor,
Thank you for your effort for removing the duplicated logic in Dxe/Smm Core and 
fixing the bugs.

Two general comments:
#1. Can you refactor your patch series in a way that the new lib code is like a "git 
move" instead of "git add"? For example, you could add an empty lib first and update 
all DSC to depend on the new lib. Then move the lib code from DxeCore to the lib folder. So that 
when reviewing the code changes, they are relative smaller.


There are enough differences between the DXE and SMM MAT logic that the
"git move" you suggest won't resolve as cleanly as you hope without
first creating separate library instances then walking them on to a
common implementation. Is there another way I can make this more
digestible?

The MAT logic is very dense, but the most complex part is the SplitTable() routine. The host-based unit test should prove that the
expected input/output of the SplitTable() function is correct in this
update.

#2. I see that you directly move the code to lib and update consumer to call 
the lib APIs. Do you think it's feasible to refine the code further such that 
the responsibilities of DxeCore and the lib can be clearer and with that the 
lib APIs can be more meaningful?


Yes, the currently exposed functions can be renamed and have more
descriptive function headers. I can also round out missing functionality
to attempt to further reduce the code footprint in the
MemoryAttributesTable.c files.

For example:

RemoveImagePropertiesRecord()
InsertImagePropertiesRecord()
DeleteImagePropertiesRecord()
CreateImagePropertiesRecord()


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107110): https://edk2.groups.io/g/devel/message/107110
Mute This Topic: https://groups.io/mt/100246933/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to