Re: [PATCH v11 0/1] ShellPkg/DynamicCommand: add HttpDynamicCommand

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

On 09/10/20 22:33, Vladimir Olovyannikov wrote:
> Hi Laszlo,
>
>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Wednesday, September 9, 2020 11:33 PM

>>> PATCH v11 changes:
>>> Address comments from Laszlo:
>>>   - use TimeBaseLib.h header to get rid of duplicated constants;
>>>   - explicitly return UINT32 in EfiTimeToEpoch().
>>
>> to be clear, I explicitly *disagree* with returning UINT32 from
>> EfiTimeToEpoch().
>>
>> I'm not "demanding" (or even suggesting) that you update the
>> EfiTimeToEpoch() implementation in this patch to return UINTN, but
>> I'd like to be very clear that, IMO, for EfiTimeToEpoch() to suffer
>> from a year 2106 problem on 64-bit systems too, is bad design. So
>> please don't list the UINT32 return type as my suggestion -- that's
>> the exact opposite of what I'd actually suggest.

> Sorry, I must have misunderstood. Do you want me to resubmit the
> patch? I am open to ideas.

Ideally:

- change the return type of EfiTimeToEpoch() to UNITN

- drop the final UINT32 cast from EfiTimeToEpoch()

- change the type of ElapsedSeconds to UINTN

- change the expression

    ElapsedSeconds > 1 ? ElapsedSeconds : 1

  to

    ElapsedSeconds > 1 ? (UINT64)ElapsedSeconds : 1

- print the expression mentioned above with the format specifier %Lu

*BUT*. These are really just small details. It would be OK to fix these
up later, incrementally. Where I see a real problem is the lack of
timely feedback from the ShellPkg maintainers.

Laszlo