Re: [edk2] [Qemu-devel] [edk2 PATCH 1/1] OvmfPkg/AcpiPlatformDxe: download ACPI tables from QEMU

Subject: Re: [edk2] [Qemu-devel] [edk2 PATCH 1/1] OvmfPkg/AcpiPlatformDxe: download ACPI tables from QEMU

From: Jordan Justen <jljusten@gmail.com>

To: Laszlo Ersek <lersek@redhat.com>

Date: 2013-11-22 21:44:14

On Fri, Nov 22, 2013 at 10:49 AM, Laszlo Ersek  wrote:
> On 11/22/13 19:10, Jordan Justen wrote:
>> Do we need to print all those fields?
>>
>> Anyway, maybe a better centralized place for this would be:
>> MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
>>
>> Also, I think we try to keep debug messages under 80 characters.
>
> We seem to think completely differently about debug messages :)

Maybe... ?

I think you can have too much debug if:
* it makes it debug builds excessively slow or large
* it makes it challenging to find items in the debug log
* it makes the code significantly more difficult to read
* it is in a code area that is just not valuable to log

> I can cut most of the fields though, and simply keep the signature (and
> remove the format string macro too, because for the signature I'd only
> use it once). Would that work for you?

Sure.

>> We do have a local InstallAcpiTable function. (Just remove "AcpiProtocol->")
>
> Yes, I saw that. But, we don't have a wrapper for the rollback on the
> error path (where we uninstall the tables). Since I used AcpiProtocol->
> in the rollback part, I wanted to be consistent here.

Good point.

-Jordan

------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing 
conversations that shape the rapidly evolving mobile landscape. Sign up now. 
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel