Re: [PATCH] UefiCpuPkg/SecCore: Use %x to print stack information

[Laszlo Ersek <lersek@redhat.com>]

On 11/15/16 18:47, Andrew Fish wrote:
> 
>> On Nov 15, 2016, at 8:59 AM, Laszlo Ersek <lersek@redhat.com
>> <mailto:lersek@redhat.com>> wrote:
>>
>> On 11/15/16 09:32, Jeff Fan wrote:
>>> SecCoreData->StackBase is VOID * type and SecCoreData->StackSize is
>>> UINTN type.
>>> We should use %x to dump their value instead of %lx.
>>>
>>> Cast pointer type to UINTN before print it.
>>>
>>> Cc: Feng Tian <feng.tian@intel.com <mailto:feng.tian@intel.com>>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com
>>> <mailto:michael.d.kinney@intel.com>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Jeff Fan <jeff.fan@intel.com <mailto:jeff.fan@intel.com>>
>>> ---
>>> UefiCpuPkg/SecCore/SecMain.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c
>>> index 2ebbc22..4d08f48 100644
>>> --- a/UefiCpuPkg/SecCore/SecMain.c
>>> +++ b/UefiCpuPkg/SecCore/SecMain.c
>>> @@ -239,9 +239,9 @@ SecStartupPhase2(
>>>
>>>   DEBUG ((
>>>     DEBUG_INFO,
>>> -    "%a() Stack Base: 0x%lx, Stack Size: 0x%lx\n",
>>> +    "%a() Stack Base: 0x%x, Stack Size: 0x%x\n",
>>>     __FUNCTION__,
>>> -    SecCoreData->StackBase,
>>> +    (UINTN) SecCoreData->StackBase,
>>>     SecCoreData->StackSize
>>>     ));
>>>
>>>
>>
>> Not disagreeing, just mentioning for completeness: we can also use %p
>> for printing (VOID*) directly.
>>
> 
> %x is sizeof(int) not sizeof(UINTN)?

Sigh, you are correct, of course. "%x" is not portable for printing UINTN.

In fact we recently discussed just that:

  [edk2] What is the right way to print a UINTN?

  https://lists.01.org/pipermail/edk2-devel/2016-September/002091.html

(There seem to be issues with my focus / attention recently... This is
now the second near-trivial issue I've missed in a couple of days. :( I
guess I should get more sleep.)

> So it seems %p would be more
> correct? Or we should just use UINT32 if that is what is intended. 
> 
> https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/PrintLib.h#L80
> 
> 	- x
> 	- The argument is an unsigned hexadecimal number. The characters used
> are 0..9 and
> 	A..F. If the flag 'L' is not specified, then the argument is assumed
> 	to be size int. This does not follow ANSI C.
> 

Right, %p is the most direct choice.

Thank you Andrew!
Laszlo