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

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

[Jeff Fan]

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>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <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
     ));
 
-- 
2.9.3.windows.2

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

[Tian, Feng]

Reviewed-by: Feng Tian <feng.tian@intel.com>

Thanks
Feng

-----Original Message-----
From: Fan, Jeff 
Sent: Tuesday, November 15, 2016 4:32 PM
To: edk2-devel@ml01.01.org
Cc: Tian, Feng <feng.tian@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [PATCH] UefiCpuPkg/SecCore: Use %x to print stack information

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>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <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
     ));
 
--
2.9.3.windows.2

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

[Laszlo Ersek]

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>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan <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.

Thanks
Laszlo

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

[Andrew Fish]

> On Nov 15, 2016, at 8:59 AM, Laszlo Ersek <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>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jeff Fan <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)? 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.


Thanks,

Andrew Fish

> Thanks
> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel <https://lists.01.org/mailman/listinfo/edk2-devel>

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

[Laszlo Ersek]

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

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

[Andrew Fish]

> On Nov 15, 2016, at 10:11 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> 
>  [edk2] What is the right way to print a UINTN?
> 
>  https://lists.01.org/pipermail/edk2-devel/2016-September/002091.html <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.)

Laszlo,

No worries that is why more eye balls is better. 

There is a lot of scientific work on mental/information overload and pilots. https://www.ucl.ac.uk/news/news-articles/170314-Information-overload-dims-our-lights

Thanks,

Andrew Fish

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

[Fan, Jeff]

Andrew,

Yes.  %P is more correct for VOID * type print. We cannot use UINT32 here because SEC could support x64 arch.

Thanks!
Jeff

From: afish@apple.com [mailto:afish@apple.com]
Sent: Wednesday, November 16, 2016 1:47 AM
To: Laszlo Ersek; Fan, Jeff; Tian, Feng; Kinney, Michael D
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] UefiCpuPkg/SecCore: Use %x to print stack information


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)? 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.




Thanks,

Andrew Fish


Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel