On 03/11/20 17:20, Leif Lindholm wrote:
> On Wed, Mar 11, 2020 at 17:14:53 +0100, Laszlo Ersek wrote:
>> On 03/11/20 16:44, Leif Lindholm wrote:
>>> One comment, not on this patch but prompted by it:
>>>
>>> On Tue, Mar 10, 2020 at 23:27:37 +0100, Laszlo Ersek wrote:
>>>> diff --git a/OvmfPkg/OvmfPkg.fdf.inc b/OvmfPkg/OvmfPkg.fdf.inc
>>>> index 66e0e4d270f5..35fd454b97ab 100644
>>>> --- a/OvmfPkg/OvmfPkg.fdf.inc
>>>> +++ b/OvmfPkg/OvmfPkg.fdf.inc
>>>> @@ -82,4 +82,10 @@
>>>
>>> I was surprised at not seeing the section header here, so had a look
>>> at the file, noticed it doesn't have any. And that all files that
>>> include it do it by:
>>>
>>> [Defines]
>>> !include OvmfPkg.fdf.inc
>>>
>>> That looks a bit error-prone and inflexible - could we move/copy the
>>> header into this file?
>>
>> No, please let us not -- I strive to keep all FDF and DSC include
>> files under OvmfPkg header-free. It gives more flexibility to the
>> includer.
>
> I see your point. The generic name suggested to me that it might be
> *intended* to hold multiple sections, and currently just happened not
> to.
>
> However, to follow a rule of least surprise...
>
>> A recent example of this was my request for NetworkPkg to expose its
>> include snippets header-less, for DSC files. Please see the "!include
>> NetworkPkg/..." directives in the OVMF DSC files; those are also by
>> design header-less:
>>
>> NetworkPkg/NetworkComponents.dsc.inc
>> NetworkPkg/NetworkDefines.dsc.inc
>> NetworkPkg/NetworkLibs.dsc.inc
>> NetworkPkg/NetworkPcds.dsc.inc
>
> ...could OvmfPkg use a similar naming scheme to this?
>
> That would also remove the drawback of not having the section name as
> part of the hunk header, as you'd have it anyway immediately above as
> part of the file name?
There are three FDF include files:
(1) DecomprScratchEnd.fdf.inc (included under [FV.FVMAIN_COMPACT], per
commit 9beac0d847bf9):
## @file
# This FDF include file computes the end of the scratch buffer used in
# DecompressMemFvs() [OvmfPkg/Sec/SecMain.c]. It is based on the decompressed
# (ie. original) size of the LZMA-compressed section of the one FFS file in
# the FVMAIN_COMPACT firmware volume.
#
# Copyright (C) 2015, Red Hat, Inc.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
##
This include file contains DEFINE and SET statements (for setting macros
and PCDs, accordingly). I don't think either a "Defines" or "Pcds"
suffix would apply.
(2) OvmfPkg.fdf.inc (included under [Defines]):
## @file
# FDF include file that defines the main macros and sets the dependent PCDs.
#
# Copyright (C) 2014, Red Hat, Inc.
# Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
Same situation (both DEFINE and SET statements).
(3) VarStore.fdf.inc (included under [FD.OVMF] and [FD.OVMF_VARS]):
## @file
# FDF include file with Layout Regions that define an empty variable store.
#
# Copyright (C) 2014, Red Hat, Inc.
# Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
I guess we could rename this to "VarStoreLayoutRegions.fdf.inc" -- but
would that really help?
I'm not against introducing helpful name suffixes, I just can't find
anything that really fits and significantly improves upon the current
names. I vaguely remember racking my brain when I was introducing these
files, for better names, and this is what I had come up with. :)
Thanks,
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#55766): https://edk2.groups.io/g/devel/message/55766
Mute This Topic: https://groups.io/mt/71867510/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
