On 8 June 2017 at 18:40, Laszlo Ersek <ler...@redhat.com> wrote: > On 06/08/17 12:11, Ard Biesheuvel wrote: >> On 7 June 2017 at 23:10, Laszlo Ersek <ler...@redhat.com> wrote: >>> On 06/06/17 20:16, Laszlo Ersek wrote: >>>> On 06/05/17 19:47, Jordan Justen wrote: >>>>> On 2017-06-03 08:42:03, Laszlo Ersek wrote: >>>>>> ... by narrower than 8-byte ADD_POINTER references. >>>>>> >>>>>> Introduce the CollectRestrictedAllocations() function, which iterates >>>>>> over >>>>> >>>>> How about Collect32BitRestrictedAllocations and similar treatment for >>>>> other names that just say 'restricted'? >>>> >>>> Something like this crossed my mind, but I didn't know how to prefix the >>>> simple variable / parameter names "RestrictedAllocations" with "32Bit", >>>> as the identifiers cannot start with a digit. >>>> >>>> I even thought of spelling it out, as in >>>> "ThirtyTwoBitRestrictedAllocations", but that seemed ridiculous. >>>> >>>> Prefixing "32Bit" with an underscore, _32Bit, looks ugly, plus the C >>>> standard actually reserves it: >>>> >>>> All identifiers that begin with an underscore are always reserved >>>> for use as identifiers with file scope in both the ordinary and tag >>>> name spaces. >>>> >>>> While I'd only use this variable name as function parameter / local >>>> variable, and thereby I'd shadow any such impl. defined global variable >>>> ("identifiers with file scope"), the shadowing would trigger a compiler >>>> warning for sure, and break the build. >>>> >>>> What do you suggest? >>> >>> Ultimately I went with >>> >>> s/RestrictedAllocations/AllocationsRestrictedTo32Bit/ >>> >>> in the patch body and in the commit message too. Cleaned up the line >>> lengths and such as well, plus retested the patch. >>> >>> Commit 4275f38507a4. >>> >> >> Hi Laszlo, >> >> Thanks again for the effort. Sadly, though, this patch is breaking my CI >> build: >> >> OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c: In function >> 'InstallQemuFwCfgTables': >> OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c:357:29: error: >> 'AllocationsRestrictedTo32Bit' may be used uninitialized in this >> function [-Werror=maybe-uninitialized] >> if (OrderedCollectionFind ( >> ^ >> OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c:975:23: note: >> 'AllocationsRestrictedTo32Bit' was declared here >> ORDERED_COLLECTION *AllocationsRestrictedTo32Bit; >> ^ >> > > Indeed, Gerd's Jenkins CI (using GCC49) reported the same (non-)issue. > > It is a compiler bug. The compiler reports that the ProcessCmdAllocate() > function may be called with "AllocationsRestrictedTo32Bit" > uninitialized. That's not the case; this function call is only reached > if CollectAllocationsRestrictedTo32Bit() returns EFI_SUCCESS -- > otherwise we jump to the "FreeLoader" label, way past the > ProcessCmdAllocate() call --, and when > CollectAllocationsRestrictedTo32Bit() succeeds, it will have > "AllocationsRestrictedTo32Bit" assigned. > > In order to expedite things, could you please help me by submitting a > one-liner patch? Namely, please set "AllocationsRestrictedTo32Bit" to > NULL right before the CollectAllocationsRestrictedTo32Bit() function call. >
Done Thanks, Ard. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel