@Laszlo,
As for type casting, I don't have strong opinions.
Space after typecast (for me) always seemed to be visually more
consistent with the rest of the code.
In my regular projects written in C-based languages, I would do it the
way you are describing.
@Vladimir
I went through the rest of the code.
There are more coding standard problems, but in terms of HTTP usage the
code seems to be OK.
Thanks,
Maciej
PS. Rabeda is my last name :)
On 19-Aug-20 11:47, Laszlo Ersek wrote:
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 (#64456): https://edk2.groups.io/g/devel/message/64456
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]
-=-=-=-=-=-=-=-=-=-=-=-
