On 08/18/20 20:33, Vladimir Olovyannikov wrote: >> -----Original Message----- >> From: Rabeda, Maciej <maciej.rab...@linux.intel.com> >> Sent: Tuesday, August 18, 2020 9:54 AM >> To: Vladimir Olovyannikov <vladimir.olovyanni...@broadcom.com>; Laszlo >> Ersek <ler...@redhat.com>; Gao, Zhichao <zhichao....@intel.com>; >> devel@edk2.groups.io >> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahm...@arm.com>; Wu, Jiaxin >> <jiaxin...@intel.com>; Fu, Siyuan <siyuan...@intel.com>; Ni, Ray >> <ray...@intel.com>; Gao, Liming <liming....@intel.com>; Nd >> <n...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
