Re: [edk2-devel] [PATCH v2 5/7] OvmfPkg/LegacyBiosDxe: Use EfiBootManagerGetBootDescription()

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

On 06/25/19 16:10, David Woodhouse wrote:
> On Tue, 2019-06-25 at 15:29 +0200, Laszlo Ersek wrote:
>> (2) You removed the NULL-initialization altogether, 
> 
> Well yeah, you told me the EDK-II coding standards forbid the common
> defensive coding technique used in the language called "C", of
> initialising the variable where it's declared. I wasn't up for arguing
> about it, so I just stopped doing it.
> 
> Now you tell me to put it back, just in a more cumbersome way... OK,
> I'll do that too.

For the record, I wrote:

    The edk2 coding style forbids initialization of locals. Please use a
    separate assignment if necessary.

I guess I didn't foresee that you'd decide it wasn't necessary after all
:) If I had thought of that, I could have dropped "if necessary".

> I'm not sure the answer is going to make me any happier... but *why*
> are we not allowed to just initialise variables where they're declared?

I don't remember. I had simply accepted this guideline years ago and
haven't gone back to question it since. There's no time to fight the
same old battles again and again. :)

>>> +
>>> +          //
>>> +          // Truncate Description and convert to ASCII.
>>> +          //
>>> +          if (StrLen (Description) >= sizeof (AsciiDescription)) {
>>> +                  Description[sizeof (AsciiDescription) - 1] = L'0';
>>
>> (3) Sneaky typo. You mean (and I requested) L'\0'. L'0' is different. :)
> 
> Oops :)
> 
> What was it I said about testing of corner cases when
> updating/rebasing? I had explicitly tested this by cutting the size
> down to 16 and watching it actually truncate. Obviously I didn't do
> *that* again this time round (I have now!)
> 
> Well spotted; thanks!
> 
> I've pushed all the fixes you've just pointed out (thanks again) to my
> 'csm' branch. Won't send another series by email today; I'll let the
> dust settle and some of the other discussions continue.

I agree, let's give Ray and others time to react. My comments have been
mostly cosmetic in this round anyway.

> When I do repost, I may send the required functional patches 1-3,7 on
> their own as I think they're ready to apply as-is, and leave the
> cosmetic patches 4-6 in a separate series for further bikeshedding.

1-3,7 only touch OvmfPkg, so I could push those quickly then.

Thanks,
Laszlo