Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test.

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

On Thu, 30 Jan 2020 at 06:08, Gaurav Jain <gaurav.jain@nxp.com> wrote:
>
> ASSERT in SetTime_Conf and SetWakeupTime_Conf Consistency Test.
> SCT Test expect return as Invalid Parameter.
> So removed ASSERT().
>

This is not all this patch does.

> Signed-off-by: Gaurav Jain <gaurav.jain@nxp.com>
> ---
>  EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> index 08fb9b0100b6..9bfb7756f0cb 100644
> --- a/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> +++ b/EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClock.c
> @@ -85,10 +85,6 @@ IsDayValid (
>    IN  EFI_TIME  *Time
>    )
>  {
> -  ASSERT (Time->Day >= 1);
> -  ASSERT (Time->Day <= mDayOfMonth[Time->Month - 1]);
> -  ASSERT (Time->Month != 2 || IsLeapYear (Time) || Time->Day <= 28);
> -
>    if (Time->Day < 1 ||
>        Time->Day > mDayOfMonth[Time->Month - 1] ||
>        (Time->Month == 2 && !IsLeapYear (Time) && Time->Day > 28)) {
> @@ -105,14 +101,15 @@ IsTimeValid(
>    )
>  {
>    // Check the input parameters are within the range specified by UEFI
> -  if (Time->Year   < 1900               ||
> -      Time->Year   > 9999               ||
> +  if (Time->Year   < 1998               ||
> +      Time->Year   > 2099               ||

That original range is based on the UEFI spec. On what basis are you
making this change?

If your RTC hardware cannot represent the original values, this is not
the place to fix that.


>        Time->Month  < 1                  ||
>        Time->Month  > 12                 ||
>        !IsDayValid (Time)                ||
>        Time->Hour   > 23                 ||
>        Time->Minute > 59                 ||
>        Time->Second > 59                 ||
> +      Time->Nanosecond > 999999999      ||
>        !IsValidTimeZone (Time->TimeZone) ||
>        !IsValidDaylight (Time->Daylight)) {
>      return FALSE;
> @@ -254,6 +251,9 @@ SetWakeupTime (
>    OUT EFI_TIME    *Time
>    )
>  {
> +  if (Time == NULL || !IsTimeValid (Time)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
>    return LibSetWakeupTime (Enabled, Time);
>  }
>
> --
> 2.17.1
>