[edk2-devel] [PATCH v5 1/1] ShellPkg/DynamicCommand: add HttpDynamicCommand

[Laszlo Ersek]

On 08/18/20 20:33, Vladimir Olovyannikov wrote:
>> -----Original Message-----
>> From: Rabeda, Maciej <maciej.rabeda at linux.intel.com>
>> Sent: Tuesday, August 18, 2020 9:54 AM
>> To: Vladimir Olovyannikov <vladimir.olovyannikov at broadcom.com>; Laszlo
>> Ersek <lersek at redhat.com>; Gao, Zhichao <zhichao.gao at intel.com>;
>> devel at edk2.groups.io
>> Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud at arm.com>; Wu, Jiaxin
>> <jiaxin.wu at intel.com>; Fu, Siyuan <siyuan.fu at intel.com>; Ni, Ray
>> <ray.ni at intel.com>; Gao, Liming <liming.gao at intel.com>; Nd
>> <nd at arm.com>
>> Subject: Re: [PATCH v5 1/1] ShellPkg/DynamicCommand: add
>> HttpDynamicCommand
>>
>> Hi Vladimir,
>>
>> I am inprog of going through the patch. There are some coding standard
>> violations.
>> For reference:
>> https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/

[...]

> OK, I will look through all files again, probably something slipped from my
> attention as I work with Linux code as well.

Now that we have ECC checking enabled in CI, submitting a personal pull
request could help. Additionally, the CI workload should be possible to
run locally, according to ".pytool/Readme.md".

(I'm planning to learn how to run that locally myself.)

> 
>> HttpApp.c:
>> Line 48: Space after type cast.
> Sorry, I don't understand here. You mean, Something = (CAST)SomethingElse
> should actually be "Something = (CAST) SomethingElse?
> I don't see this say in the Tftp.c:
> if (AsciiStrnCmp ((CHAR8 *)Option->OptionStr, "tsize", 5) == 0) ...
> or in TftpApp.c
> Status = (EFI_STATUS)RunTftp (ImageHandle, SystemTable);
> Am I missing anything?


In core edk2 packages, casts are frequently written like this:

  (TYPE) Expression

(Importantly, Expression itself is not parenthesized.)

In my opinion, this is a bad practice. That's because the typecast has
one of the strongest operator bindings in the C language, and visually
distancing (TYPE) from "Expression" suggests the opposite -- it's
counter-intuitive.

I strongly prefer

  (TYPE)Expression

but other maintainers may have a different preference.

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#64423): https://edk2.groups.io/g/devel/message/64423
Mute This Topic: https://groups.io/mt/75826968/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-