Re: [PATCH v10 1/1] ShellPkg/DynamicCommand: add HttpDynamicCommand

["Laszlo Ersek" <lersek@redhat.com>]

On 09/04/20 19:55, Vladimir Olovyannikov wrote:

> There is also another issue with the TimebaseLib: inconsistency in return
> values of the
> EfiTimeToEpoch (returns UINT32, should return UINTN, as Zhichao pointed out
> earlier in the previous HttpDynamicCommand patchset).
> If this one is fixed, I can just use the TimeBaseLib.h header for constants.

Consuming TimeBaseLib.h in this patch would be really nice.

There are two EfiTimeToEpoch() call sites in edk2:

ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c

The latter stores the return value in a UINTN variable, so that seems OK. The former is a bit messier, but it seems to ensure that the result fits in 32 bits for HW reasons anyway:

  // 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;
  }
...
  EpochSeconds = EfiTimeToEpoch (Time);
...
  MmioWrite32 (mPL031RtcBase + PL031_RTC_LR_LOAD_REGISTER, EpochSeconds);

So I think we'd need two patches:

(1) add an explicit (UINT32) cast to the EfiTimeToEpoch() call in "ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c",

(2) change the return value to UINTN in "EmbeddedPkg/Include/Library/TimeBaseLib.h" and "EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.c".

Hmm wait... There are five more call sites in edk2-platforms. :( OK, I give up here. Sorry.

Laszlo