[PATCH 0/2] EmbeddedPkg/TimeBaseLib: remove useless truncation to 32-bit

[PATCH 0/2] EmbeddedPkg/TimeBaseLib: remove useless truncation to 32-bit

[Laszlo Ersek]

Repo:   https://pagure.io/lersek/edk2.git
Branch: timebaselib_uintn

Through the virtio-fs driver, EmbeddedPkg/TimeBaseLib got exposed to
VS2019 for the first time. VS2019 flagged an arguably unintended,
implicit UINTN->UINT32 conversion in EfiTimeToEpoch(); let's remedy
that.

Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks!
Laszlo

Laszlo Ersek (2):
  ArmPlatformPkg/PL031RealTimeClockLib: cast EfiTimeToEpoch() val. to
    UINT32
  EmbeddedPkg/TimeBaseLib: remove useless truncation to 32-bit

 EmbeddedPkg/Include/Library/TimeBaseLib.h                            | 2 +-
 ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c | 2 +-
 EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c                        | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.19.1.3.g30247aa5d201

[PATCH 1/2] ArmPlatformPkg/PL031RealTimeClockLib: cast EfiTimeToEpoch() val. to UINT32

[Laszlo Ersek]

In preparation for changing EfiTimeToEpoch()'s return type to UINTN, cast
EfiTimeToEpoch()'s retval to UINT32 explicitly, in LibSetTime().

Currently, this is a no-op, and even after widening the retval, it will
make no difference, as LibSetTime() explicitly restricts Time->Year under
2106, given that "the PL031 is a 32-bit counter counting seconds". The
patch is made for preventing compiler warnings.

Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
index 0df4ceb1cb39..3afef4bf9ad4 100644
--- a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
+++ b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
@@ -180,53 +180,53 @@ EFIAPI
 LibSetTime (
   IN  EFI_TIME                *Time
   )
 {
   EFI_STATUS  Status;
   UINT32      EpochSeconds;
 
   // Because the PL031 is a 32-bit counter counting seconds,
   // the maximum time span is just over 136 years.
   // Time is stored in Unix Epoch format, so it starts in 1970,
   // Therefore it can not exceed the year 2106.
   if ((Time->Year < 1970) || (Time->Year >= 2106)) {
     return EFI_UNSUPPORTED;
   }
 
   // Initialize the hardware if not already done
   if (!mPL031Initialized) {
     Status = InitializePL031 ();
     if (EFI_ERROR (Status)) {
       return Status;
     }
   }
 
-  EpochSeconds = EfiTimeToEpoch (Time);
+  EpochSeconds = (UINT32)EfiTimeToEpoch (Time);
 
   // Adjust for the correct time zone, i.e. convert to UTC time zone
   // The timezone setting also reflects the DST setting of the clock
   if (Time->TimeZone != EFI_UNSPECIFIED_TIMEZONE) {
     EpochSeconds -= Time->TimeZone * SEC_PER_MIN;
   } else if ((Time->Daylight & EFI_TIME_IN_DAYLIGHT) == EFI_TIME_IN_DAYLIGHT) {
     // Convert to un-adjusted time, i.e. fall back one hour
     EpochSeconds -= SEC_PER_HOUR;
   }
 
   // Set the PL031
   MmioWrite32 (mPL031RtcBase + PL031_RTC_LR_LOAD_REGISTER, EpochSeconds);
 
   return EFI_SUCCESS;
 }
 
 
 /**
   Returns the current wakeup alarm clock setting.
 
   @param  Enabled               Indicates if the alarm is currently enabled or disabled.
   @param  Pending               Indicates if the alarm signal is pending and requires acknowledgement.
   @param  Time                  The current alarm setting.
 
   @retval EFI_SUCCESS           The alarm settings were returned.
   @retval EFI_INVALID_PARAMETER Any parameter is NULL.
   @retval EFI_DEVICE_ERROR      The wakeup time could not be retrieved due to a hardware error.
 
 **/
-- 
2.19.1.3.g30247aa5d201

[PATCH 2/2] EmbeddedPkg/TimeBaseLib: remove useless truncation to 32-bit

[Laszlo Ersek]

EfiTimeToEpoch() calls EfiGetEpochDays() internally, which (reasonably)
returns a UINTN. But then EfiTimeToEpoch() truncates the EfiGetEpochDays()
retval to UINT32 for no good reason, effectively restricting Time->Year
under 2106.

This truncation was pointed out with a valid warning (= build error) by
VS2019.

Allow EfiTimeToEpoch() to return / propagate a UINTN value.

Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    This patch may only be committed after the edk2-platforms patch
    "Silicon/Marvell/RealTimeClockLib: make EpochSeconds, WakeupSeconds
    UINTN".

 EmbeddedPkg/Include/Library/TimeBaseLib.h     | 2 +-
 EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/EmbeddedPkg/Include/Library/TimeBaseLib.h b/EmbeddedPkg/Include/Library/TimeBaseLib.h
index 3c2d3660c66c..90853c3f4b93 100644
--- a/EmbeddedPkg/Include/Library/TimeBaseLib.h
+++ b/EmbeddedPkg/Include/Library/TimeBaseLib.h
@@ -83,7 +83,7 @@ EpochToEfiTime (
 /**
   Converts EFI_TIME to Epoch seconds (elapsed since 1970 JANUARY 01, 00:00:00 UTC)
  **/
-UINT32
+UINTN
 EFIAPI
 EfiTimeToEpoch (
   IN  EFI_TIME  *Time
diff --git a/EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c b/EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c
index 136ce8a51e86..78fc7b6cd2e5 100644
--- a/EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c
+++ b/EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c
@@ -97,24 +97,24 @@ EfiGetEpochDays (
   return EpochDays;
 }
 /**
   Converts EFI_TIME to Epoch seconds (elapsed since 1970 JANUARY 01, 00:00:00 UTC)
  **/
-UINT32
+UINTN
 EFIAPI
 EfiTimeToEpoch (
   IN  EFI_TIME  *Time
   )
 {
-  UINT32 EpochDays;   // Number of days elapsed since EPOCH_JULIAN_DAY
-  UINT32 EpochSeconds;
+  UINTN EpochDays;   // Number of days elapsed since EPOCH_JULIAN_DAY
+  UINTN EpochSeconds;
 
   EpochDays = EfiGetEpochDays (Time);
 
   EpochSeconds = (EpochDays * SEC_PER_DAY) + ((UINTN)Time->Hour * SEC_PER_HOUR) + (Time->Minute * SEC_PER_MIN) + Time->Second;
 
   return EpochSeconds;
 }
 
 /**
   returns Day of the week [0-6] 0=Sunday
  **/
-- 
2.19.1.3.g30247aa5d201

Re: [PATCH 0/2] EmbeddedPkg/TimeBaseLib: remove useless truncation to 32-bit

[Ard Biesheuvel]

On 12/21/20 12:36 PM, Laszlo Ersek wrote:
> Repo:   https://pagure.io/lersek/edk2.git
> Branch: timebaselib_uintn
> 
> Through the virtio-fs driver, EmbeddedPkg/TimeBaseLib got exposed to
> VS2019 for the first time. VS2019 flagged an arguably unintended,
> implicit UINTN->UINT32 conversion in EfiTimeToEpoch(); let's remedy
> that.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> 

Acked-by: Ard Biesheuvel <ard.biesheuvel@arm.com>



> 
> Laszlo Ersek (2):
>   ArmPlatformPkg/PL031RealTimeClockLib: cast EfiTimeToEpoch() val. to
>     UINT32
>   EmbeddedPkg/TimeBaseLib: remove useless truncation to 32-bit
> 
>  EmbeddedPkg/Include/Library/TimeBaseLib.h                            | 2 +-
>  ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c | 2 +-
>  EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c                        | 6 +++---
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 

Re: [edk2-devel] [PATCH 0/2] EmbeddedPkg/TimeBaseLib: remove useless truncation to 32-bit

[Laszlo Ersek]

On 12/21/20 13:17, Ard Biesheuvel wrote:
> On 12/21/20 12:36 PM, Laszlo Ersek wrote:
>> Repo:   https://pagure.io/lersek/edk2.git
>> Branch: timebaselib_uintn
>>
>> Through the virtio-fs driver, EmbeddedPkg/TimeBaseLib got exposed to
>> VS2019 for the first time. VS2019 flagged an arguably unintended,
>> implicit UINTN->UINT32 conversion in EfiTimeToEpoch(); let's remedy
>> that.
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Cc: Leif Lindholm <leif@nuviainc.com>
>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
> 
> Acked-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

Merged as commit range 3ce3274a5ea4..c06635ea3f4b, via
<https://github.com/tianocore/edk2/pull/1254>.

Thank you!
Laszlo


>> Laszlo Ersek (2):
>>   ArmPlatformPkg/PL031RealTimeClockLib: cast EfiTimeToEpoch() val. to
>>     UINT32
>>   EmbeddedPkg/TimeBaseLib: remove useless truncation to 32-bit
>>
>>  EmbeddedPkg/Include/Library/TimeBaseLib.h                            | 2 +-
>>  ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c | 2 +-
>>  EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c                        | 6 +++---
>>  3 files changed, 5 insertions(+), 5 deletions(-)
>>
> 
> 
> 
> 
> 
>