* [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test. @ 2020-01-30 5:08 Gaurav Jain 2020-01-30 9:21 ` Ard Biesheuvel 0 siblings, 1 reply; 8+ messages in thread From: Gaurav Jain @ 2020-01-30 5:08 UTC (permalink / raw) To: devel@edk2.groups.io Cc: Leif Lindholm, Ard Biesheuvel, Pankaj Bansal, Gaurav Jain ASSERT in SetTime_Conf and SetWakeupTime_Conf Consistency Test. SCT Test expect return as Invalid Parameter. So removed ASSERT(). 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 || 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test. 2020-01-30 5:08 [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test Gaurav Jain @ 2020-01-30 9:21 ` Ard Biesheuvel 2020-01-31 8:27 ` [EXT] " gaurav.jain 0 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2020-01-30 9:21 UTC (permalink / raw) To: Gaurav Jain; +Cc: devel@edk2.groups.io, Leif Lindholm, Pankaj Bansal 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test. 2020-01-30 9:21 ` Ard Biesheuvel @ 2020-01-31 8:27 ` gaurav.jain 2020-02-11 10:37 ` FW: " Gaurav Jain 0 siblings, 1 reply; 8+ messages in thread From: gaurav.jain @ 2020-01-31 8:27 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: devel@edk2.groups.io, Leif Lindholm, Pankaj Bansal > -----Original Message----- > From: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Sent: Thursday, January 30, 2020 2:52 PM > To: Gaurav Jain <gaurav.jain@nxp.com> > Cc: devel@edk2.groups.io; Leif Lindholm <leif@nuviainc.com>; Pankaj Bansal > <pankaj.bansal@nxp.com> > Subject: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime > Services test. > > Caution: EXT Email > > 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. As per the UEFI SCT Test, SetWakeupTime_Conf expect EFI_INVALID_PARAMETER for Time.Year is 1997 and 2100. Below is the link to check the Test code https://github.com/tianocore/edk2-test/blob/master/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/TimeServices/BlackBoxTest/TimeServicesBBTestConformance.c (Line: 847) Either UEFI spec need to be modified as per the test or SCT Test needs fix as per UEFI Specification. > > > > 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 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* FW: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test. 2020-01-31 8:27 ` [EXT] " gaurav.jain @ 2020-02-11 10:37 ` Gaurav Jain 2020-02-11 14:19 ` Ard Biesheuvel 0 siblings, 1 reply; 8+ messages in thread From: Gaurav Jain @ 2020-02-11 10:37 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: devel@edk2.groups.io, Leif Lindholm, Pankaj Bansal Hi Ard I am waiting for your response. Regards Gaurav -----Original Message----- From: Gaurav Jain Sent: Friday, January 31, 2020 1:58 PM To: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: devel@edk2.groups.io; Leif Lindholm <leif@nuviainc.com>; Pankaj Bansal <pankaj.bansal@nxp.com> Subject: RE: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test. > -----Original Message----- > From: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Sent: Thursday, January 30, 2020 2:52 PM > To: Gaurav Jain <gaurav.jain@nxp.com> > Cc: devel@edk2.groups.io; Leif Lindholm <leif@nuviainc.com>; Pankaj > Bansal <pankaj.bansal@nxp.com> > Subject: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT > Runtime Services test. > > Caution: EXT Email > > 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. As per the UEFI SCT Test, SetWakeupTime_Conf expect EFI_INVALID_PARAMETER for Time.Year is 1997 and 2100. Below is the link to check the Test code https://github.com/tianocore/edk2-test/blob/master/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/TimeServices/BlackBoxTest/TimeServicesBBTestConformance.c (Line: 847) Either UEFI spec need to be modified as per the test or SCT Test needs fix as per UEFI Specification. > > > > 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 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: FW: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test. 2020-02-11 10:37 ` FW: " Gaurav Jain @ 2020-02-11 14:19 ` Ard Biesheuvel 2020-02-17 6:25 ` Gaurav Jain 0 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2020-02-11 14:19 UTC (permalink / raw) To: Gaurav Jain; +Cc: devel@edk2.groups.io, Leif Lindholm, Pankaj Bansal On Tue, 11 Feb 2020 at 11:37, Gaurav Jain <gaurav.jain@nxp.com> wrote: > > Hi Ard > > I am waiting for your response. > You said > Either UEFI spec need to be modified as per the test or SCT Test needs fix as per UEFI Specification. > so you answered your own question, no? > -----Original Message----- > From: Gaurav Jain > Sent: Friday, January 31, 2020 1:58 PM > To: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: devel@edk2.groups.io; Leif Lindholm <leif@nuviainc.com>; Pankaj Bansal <pankaj.bansal@nxp.com> > Subject: RE: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test. > > > > > -----Original Message----- > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Sent: Thursday, January 30, 2020 2:52 PM > > To: Gaurav Jain <gaurav.jain@nxp.com> > > Cc: devel@edk2.groups.io; Leif Lindholm <leif@nuviainc.com>; Pankaj > > Bansal <pankaj.bansal@nxp.com> > > Subject: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT > > Runtime Services test. > > > > Caution: EXT Email > > > > 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. > > As per the UEFI SCT Test, SetWakeupTime_Conf expect EFI_INVALID_PARAMETER for Time.Year is 1997 and 2100. > Below is the link to check the Test code https://github.com/tianocore/edk2-test/blob/master/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/TimeServices/BlackBoxTest/TimeServicesBBTestConformance.c (Line: 847) > > Either UEFI spec need to be modified as per the test or SCT Test needs fix as per UEFI Specification. > > > > > > > > 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 > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: FW: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test. 2020-02-11 14:19 ` Ard Biesheuvel @ 2020-02-17 6:25 ` Gaurav Jain 2020-02-17 6:56 ` Ard Biesheuvel 0 siblings, 1 reply; 8+ messages in thread From: Gaurav Jain @ 2020-02-17 6:25 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: devel@edk2.groups.io, Leif Lindholm, Pankaj Bansal Hi Ard I have also checked UEFI Shell Specification Version 2.2(https://uefi.org/sites/default/files/resources/UEFI_Shell_2_2.pdf) For date command the range of valid years is from 1998–2099 (on page 121) So Range of valid years is conflicting in UEFI Spec and UEFI Shell Spec. I think UEFI spec needs to be corrected. What are your thoughts? > -----Original Message----- > From: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Sent: Tuesday, February 11, 2020 7:50 PM > To: Gaurav Jain <gaurav.jain@nxp.com> > Cc: devel@edk2.groups.io; Leif Lindholm <leif@nuviainc.com>; Pankaj Bansal > <pankaj.bansal@nxp.com> > Subject: Re: FW: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT > Runtime Services test. > > Caution: EXT Email > > On Tue, 11 Feb 2020 at 11:37, Gaurav Jain <gaurav.jain@nxp.com> wrote: > > > > Hi Ard > > > > I am waiting for your response. > > > > You said > > > Either UEFI spec need to be modified as per the test or SCT Test needs fix as > per UEFI Specification. > > > > so you answered your own question, no? > > > -----Original Message----- > > From: Gaurav Jain > > Sent: Friday, January 31, 2020 1:58 PM > > To: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Cc: devel@edk2.groups.io; Leif Lindholm <leif@nuviainc.com>; Pankaj > > Bansal <pankaj.bansal@nxp.com> > > Subject: RE: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT > Runtime Services test. > > > > > > > > > -----Original Message----- > > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > Sent: Thursday, January 30, 2020 2:52 PM > > > To: Gaurav Jain <gaurav.jain@nxp.com> > > > Cc: devel@edk2.groups.io; Leif Lindholm <leif@nuviainc.com>; Pankaj > > > Bansal <pankaj.bansal@nxp.com> > > > Subject: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT > > > Runtime Services test. > > > > > > Caution: EXT Email > > > > > > 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. > > > > As per the UEFI SCT Test, SetWakeupTime_Conf expect > EFI_INVALID_PARAMETER for Time.Year is 1997 and 2100. > > Below is the link to check the Test code > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith > > ub.com%2Ftianocore%2Fedk2-test%2Fblob%2Fmaster%2Fuefi- > sct%2FSctPkg%2FT > > > estCase%2FUEFI%2FEFI%2FRuntimeServices%2FTimeServices%2FBlackBoxTes > t%2 > > > FTimeServicesBBTestConformance.c&data=02%7C01%7Cgaurav.jain%40 > nxp. > > > com%7C3c6e62107ab149b8709808d7aefd73e8%7C686ea1d3bc2b4c6fa92cd9 > 9c5c301 > > > 635%7C0%7C0%7C637170275901140244&sdata=83GGnHy%2BZvzz5yZo > 8et56FQqH > > meYimYGB9dJxtlluKM%3D&reserved=0 (Line: 847) > > > > Either UEFI spec need to be modified as per the test or SCT Test needs fix as > per UEFI Specification. > > > > > > > > > > > > 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 > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: FW: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test. 2020-02-17 6:25 ` Gaurav Jain @ 2020-02-17 6:56 ` Ard Biesheuvel 2020-02-17 7:02 ` Ard Biesheuvel 0 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2020-02-17 6:56 UTC (permalink / raw) To: Gaurav Jain; +Cc: devel@edk2.groups.io, Leif Lindholm, Pankaj Bansal On Mon, 17 Feb 2020 at 07:25, Gaurav Jain <gaurav.jain@nxp.com> wrote: > > Hi Ard > > I have also checked UEFI Shell Specification Version 2.2(https://uefi.org/sites/default/files/resources/UEFI_Shell_2_2.pdf) > For date command the range of valid years is from 1998–2099 (on page 121) > > So Range of valid years is conflicting in UEFI Spec and UEFI Shell Spec. > I think UEFI spec needs to be corrected. What are your thoughts? > Page 121 does not mention data. Page 111 does, and tells me that a two-digit year is interpreted as 199x or 20xx, and a four digit year is interpreted as is. How exactly does that conflict with the UEFI spec? > > > -----Original Message----- > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Sent: Tuesday, February 11, 2020 7:50 PM > > To: Gaurav Jain <gaurav.jain@nxp.com> > > Cc: devel@edk2.groups.io; Leif Lindholm <leif@nuviainc.com>; Pankaj Bansal > > <pankaj.bansal@nxp.com> > > Subject: Re: FW: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT > > Runtime Services test. > > > > Caution: EXT Email > > > > On Tue, 11 Feb 2020 at 11:37, Gaurav Jain <gaurav.jain@nxp.com> wrote: > > > > > > Hi Ard > > > > > > I am waiting for your response. > > > > > > > You said > > > > > Either UEFI spec need to be modified as per the test or SCT Test needs fix as > > per UEFI Specification. > > > > > > > so you answered your own question, no? > > > > > -----Original Message----- > > > From: Gaurav Jain > > > Sent: Friday, January 31, 2020 1:58 PM > > > To: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > Cc: devel@edk2.groups.io; Leif Lindholm <leif@nuviainc.com>; Pankaj > > > Bansal <pankaj.bansal@nxp.com> > > > Subject: RE: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT > > Runtime Services test. > > > > > > > > > > > > > -----Original Message----- > > > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > > Sent: Thursday, January 30, 2020 2:52 PM > > > > To: Gaurav Jain <gaurav.jain@nxp.com> > > > > Cc: devel@edk2.groups.io; Leif Lindholm <leif@nuviainc.com>; Pankaj > > > > Bansal <pankaj.bansal@nxp.com> > > > > Subject: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT > > > > Runtime Services test. > > > > > > > > Caution: EXT Email > > > > > > > > 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. > > > > > > As per the UEFI SCT Test, SetWakeupTime_Conf expect > > EFI_INVALID_PARAMETER for Time.Year is 1997 and 2100. > > > Below is the link to check the Test code > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith > > > ub.com%2Ftianocore%2Fedk2-test%2Fblob%2Fmaster%2Fuefi- > > sct%2FSctPkg%2FT > > > > > estCase%2FUEFI%2FEFI%2FRuntimeServices%2FTimeServices%2FBlackBoxTes > > t%2 > > > > > FTimeServicesBBTestConformance.c&data=02%7C01%7Cgaurav.jain%40 > > nxp. > > > > > com%7C3c6e62107ab149b8709808d7aefd73e8%7C686ea1d3bc2b4c6fa92cd9 > > 9c5c301 > > > > > 635%7C0%7C0%7C637170275901140244&sdata=83GGnHy%2BZvzz5yZo > > 8et56FQqH > > > meYimYGB9dJxtlluKM%3D&reserved=0 (Line: 847) > > > > > > Either UEFI spec need to be modified as per the test or SCT Test needs fix as > > per UEFI Specification. > > > > > > > > > > > > > > > > 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 > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: FW: [EXT] Re: [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test. 2020-02-17 6:56 ` Ard Biesheuvel @ 2020-02-17 7:02 ` Ard Biesheuvel 0 siblings, 0 replies; 8+ messages in thread From: Ard Biesheuvel @ 2020-02-17 7:02 UTC (permalink / raw) To: Gaurav Jain; +Cc: devel@edk2.groups.io, Leif Lindholm, Pankaj Bansal On Mon, 17 Feb 2020 at 07:56, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On Mon, 17 Feb 2020 at 07:25, Gaurav Jain <gaurav.jain@nxp.com> wrote: > > > > Hi Ard > > > > I have also checked UEFI Shell Specification Version 2.2(https://uefi.org/sites/default/files/resources/UEFI_Shell_2_2.pdf) > > For date command the range of valid years is from 1998–2099 (on page 121) > > > > So Range of valid years is conflicting in UEFI Spec and UEFI Shell Spec. > > I think UEFI spec needs to be corrected. What are your thoughts? > > > > Page 121 does not mention data. Page 111 does, and tells me that a > two-digit year is interpreted as 199x or 20xx, and a four digit year > is interpreted as is. > > How exactly does that conflict with the UEFI spec? > OK, so I found this on page 111: """The range of valid years is from 1998–2099""" and so indeed, the Shell 'date' command's year range is narrower than the UEFI spec's, but that does not make it non-compliant. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-02-17 7:02 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-01-30 5:08 [PATCH 1/1] EmbeddedPkg: Fixed Asserts in SCT Runtime Services test Gaurav Jain 2020-01-30 9:21 ` Ard Biesheuvel 2020-01-31 8:27 ` [EXT] " gaurav.jain 2020-02-11 10:37 ` FW: " Gaurav Jain 2020-02-11 14:19 ` Ard Biesheuvel 2020-02-17 6:25 ` Gaurav Jain 2020-02-17 6:56 ` Ard Biesheuvel 2020-02-17 7:02 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox