From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f47.google.com (mail-wm1-f47.google.com [209.85.128.47]) by mx.groups.io with SMTP id smtpd.web11.7605.1609938220291914092 for ; Wed, 06 Jan 2021 05:03:40 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=dbmoxoT8; spf=pass (domain: nuviainc.com, ip: 209.85.128.47, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f47.google.com with SMTP id 190so2389872wmz.0 for ; Wed, 06 Jan 2021 05:03:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=d3MmwJEOwpoTnAT2wTF76BosYP2Mv6N8GbwTakjOLfg=; b=dbmoxoT8h1eGos8dZt1YsynoEkvrBcJk+L6LKSoDkc1XEL2cgaNJ7PtetPMfcFUoYX dhh/EpKQk7RDLf9dC91PWx++7mJbj03xBshBxmAEXAwN2eWPUaADkfig2vJmtWf8hxMJ Dv01HfxIkkm7/rAbeZZ+nU5s2yClQ1zbKiVAaz/9HsWwSNDKwgJC8jGDj3X1jc+W3Liw QfmEmb47BUSX7CapxajXLKCx8osNudhf6jZNGCBz0/On0HNlEYsTZ7v4WapXdKsyFZCZ L9NwtLrWh+B2bxtzVn1PKNbyQfKp16YGeUmCwtj7+R0N/Pyg4jPWJ9/Av67i4I0phZ6e +2WA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=d3MmwJEOwpoTnAT2wTF76BosYP2Mv6N8GbwTakjOLfg=; b=SeprufvQid9cEMHd6Av69oRY8Q74tBqtRGYhneregDciJOuBaTVc3Se3V3wGhdTQXp YuwH/pt3r5PL8rw7GyQrO+qwpQA0v9TRfVAhJzgKVgtvdc/88VZWNeLShMRIpm44+oWk G/ayPY5jZCX8Ab7ajKuuMtCrc7zNHsKlhZ6zo4zSn34ghrqyvRANINpTo+Sy73I0W1G4 Ueer15hQsG6uzP6ld5xg2ejbgcC5Db4hgero6FIqo8tx7/rt1n6T0T13sOwFBGsYC5tC f00n6aLQtQSXdz9lVSNPvm0qNXGyHTFpsjqJlavE8gjowusSvd7WyFwlQRXGX/8iJDXl ne7w== X-Gm-Message-State: AOAM531TK2JivyUegxbtaMnpa4j5Ok13v7ea8pj5Nuo5Z0VK0Gbisq+j H8Z8M+ECRDWpotyBNUwFtcaaHA== X-Google-Smtp-Source: ABdhPJwYKrpqMCTPg5ZB0zc8hoatOb7wCgOSW1FGzBOK+RfMsDBOA7ywumOOTm+YcWzD+zS3qcC/Yg== X-Received: by 2002:a1c:b788:: with SMTP id h130mr3586506wmf.94.1609938218755; Wed, 06 Jan 2021 05:03:38 -0800 (PST) Return-Path: Received: from vanye (cpc1-cmbg19-2-0-cust915.5-4.cable.virginm.net. [82.27.183.148]) by smtp.gmail.com with ESMTPSA id g192sm2935599wme.48.2021.01.06.05.03.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Jan 2021 05:03:38 -0800 (PST) Date: Wed, 6 Jan 2021 13:03:35 +0000 From: "Leif Lindholm" To: Nhi Pham Cc: devel@edk2.groups.io, Ard Biesheuvel Subject: Re: [PATCH 1/2] EmbeddedPkg/TimeBaseLib: Add function to check Timezone and Daylight Message-ID: <20210106130335.GB1664@vanye> References: <20210106105558.9582-1-nhi@os.amperecomputing.com> <20210106105558.9582-2-nhi@os.amperecomputing.com> MIME-Version: 1.0 In-Reply-To: <20210106105558.9582-2-nhi@os.amperecomputing.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Nhi, On Wed, Jan 06, 2021 at 17:55:57 +0700, Nhi Pham wrote: > This adds two functions IsValidTimeZone() and IsValidDaylight() to check > the time zone and daylight value from EFI time. These functions are > retrieved from the RealTimeClockRuntimeDxe module as they reduce > duplicated code in RTC modules. > > Cc: Leif Lindholm > Cc: Ard Biesheuvel > Signed-off-by: Nhi Pham > --- > EmbeddedPkg/Include/Library/TimeBaseLib.h | 13 ++++++ > EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c | 47 ++++++++++++++------ > 2 files changed, 47 insertions(+), 13 deletions(-) > > diff --git a/EmbeddedPkg/Include/Library/TimeBaseLib.h b/EmbeddedPkg/Include/Library/TimeBaseLib.h > index 90853c3f4b93..8bebf5886db8 100644 > --- a/EmbeddedPkg/Include/Library/TimeBaseLib.h > +++ b/EmbeddedPkg/Include/Library/TimeBaseLib.h > @@ -2,6 +2,7 @@ > * > * Copyright (c) 2016, Hisilicon Limited. All rights reserved. > * Copyright (c) 2016-2019, Linaro Limited. All rights reserved. > +* Copyright (c) 2020, Ampere Computing LLC. All rights reserved. 2021? :) > * > * SPDX-License-Identifier: BSD-2-Clause-Patent > * > @@ -64,6 +65,18 @@ IsDayValid ( > IN EFI_TIME *Time > ); > > +BOOLEAN > +EFIAPI > +IsValidTimeZone ( > + IN INT16 TimeZone > + ); > + > +BOOLEAN > +EFIAPI > +IsValidDaylight ( > + IN INT8 Daylight > + ); > + Could you please add doxygen comment blocks to these new functions (and repeat them in the .c)? I know we've been lax about this in the past, but I would like for us to start improving (especially in common libraries). > BOOLEAN > EFIAPI > IsTimeValid ( > diff --git a/EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c b/EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c > index 78fc7b6cd2e5..02d9901338b9 100644 > --- a/EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c > +++ b/EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c > @@ -2,6 +2,7 @@ > * > * Copyright (c) 2016, Hisilicon Limited. All rights reserved. > * Copyright (c) 2016-2019, Linaro Limited. All rights reserved. > +* Copyright (c) 2020, Ampere Computing LLC. All rights reserved. > * > * SPDX-License-Identifier: BSD-2-Clause-Patent > * > @@ -173,23 +174,43 @@ IsDayValid ( > > BOOLEAN > EFIAPI > -IsTimeValid( > +IsValidTimeZone ( > + IN INT16 TimeZone > + ) > +{ > + return TimeZone == EFI_UNSPECIFIED_TIMEZONE || > + (TimeZone >= -1440 && TimeZone <= 1440); > +} > + > +BOOLEAN > +EFIAPI > +IsValidDaylight ( > + IN INT8 Daylight > + ) > +{ > + return Daylight == 0 || > + Daylight == EFI_TIME_ADJUST_DAYLIGHT || > + Daylight == (EFI_TIME_ADJUST_DAYLIGHT | EFI_TIME_IN_DAYLIGHT); > +} > + > +BOOLEAN > +EFIAPI > +IsTimeValid ( > IN EFI_TIME *Time > ) > { > // Check the input parameters are within the range specified by UEFI > - if ((Time->Year < 2000) || > - (Time->Year > 2099) || > - (Time->Month < 1 ) || > - (Time->Month > 12 ) || > - (!IsDayValid (Time) ) || > - (Time->Hour > 23 ) || > - (Time->Minute > 59 ) || > - (Time->Second > 59 ) || > - (Time->Nanosecond > 999999999) || > - (!((Time->TimeZone == EFI_UNSPECIFIED_TIMEZONE) || ((Time->TimeZone >= -1440) && (Time->TimeZone <= 1440)))) || > - (Time->Daylight & (~(EFI_TIME_ADJUST_DAYLIGHT | EFI_TIME_IN_DAYLIGHT))) > - ) { > + if ((Time->Year < 2000) || > + (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))) { Can you split the tidying of unchanged lines up into a separate preceding patch please? Best Regards, Leif p.s. I have now cleared my backlog after getting back from Christmas and am just about to get back to reviewing the massive mt-jade platform port. Apologies for the delay on this. > return FALSE; > } > > -- > 2.17.1 >