Hi Maciej,
[snipping liberally, comments below]
On 06/25/21 16:56, Rabeda, Maciej wrote:
> On 22-Jun-21 17:57, Laszlo Ersek wrote:
>> On 06/11/21 13:54, Rabeda, Maciej wrote:
>>> On 08-Jun-21 15:06, Laszlo Ersek wrote:
>>>> +typedef struct {
>>>> + UINT8 Algorithm; //
>>>> ISCSI_CHAP_ALGORITHM_*, CHAP_A
>>> Any chance to define an enum type for Algorithm? IMHO it brings more
>>> meaning to that number and limits the set of values it is expected
>>> to have.
>> I picked UINT8 intentionally; I wanted to stay close to the
>> definition at
>>
>> https://www.iana.org/assignments/ppp-numbers/ppp-numbers.xhtml#ppp-numbers-9
>>
>>
>> which says, "A one octet field". (Hence the reference to "CHAP_A" in
>> the comment as well.)
>>
>> If we want an enum here, then I need to prepend a patch, for first
>> replacing the existent ISCSI_CHAP_ALGORITHM_MD5 macro with an enum
>> type / constant.
>>
>> I still like UINT8 for this, but if you strongly prefer the enum, I
>> can certainly do it. Please advise :)
> Okay, let's stick to UINT8 based on IANA definition.
Thanks!
>>>> - if (CompareMem (VerifyRsp, TargetResponse, MD5_DIGEST_SIZE) !=
>>>> 0) {
>>>> + if (CompareMem (VerifyRsp, TargetResponse,
>>>> + AuthData->Hash->DigestSize) != 0) {
>>>> Status = EFI_SECURITY_VIOLATION;
>>>> }
>>> Either break like regular function call or leave it in a single line
>>> for readability, since UEFI coding standard section 5.1.1 allows for
>>> lines up to 120. I opt for the latter, since param break will look
>>> ugly, but if it looks better in your editor than a line over 80
>>> chars in size - go for it.
>> My mistake, sorry -- I forgot that the "condensed style" that we
>> actually prefer in ArmVirtPkg and OvmfPkg is not accepted in other
>> packages.
> Making it the other way around. Apart from historical reasons (EDK2
> coding style evolving) - why is a different style accepted in other
> packages? We have a EDK2 coding style document for a reason.
> If it is not perfect, it does not suit our needs or simply hurts our
> eyes (which 'param per line' in if statements does to me), we can try
> to change it to the "condensed style".
* Submitted on 11 Aug 2017:
[edk2] [edk2-CCodingStandardsSpecification PATCH 0/2] improvements related to
line wrapping
[edk2] [edk2-CCodingStandardsSpecification PATCH 1/2] Source Files / General
Rules: limit line lengths to 80 columns
[edk2] [edk2-CCodingStandardsSpecification PATCH 2/2] Source Files / Spacing
/ Multi-line func. calls: allow condensed arguments
http://mid.mail-archive.com/20170811164851.9466-1-lersek@redhat.com
http://mid.mail-archive.com/20170811164851.9466-2-lersek@redhat.com
http://mid.mail-archive.com/20170811164851.9466-3-lersek@redhat.com
The condensed arguments style is described in patch 2/2.
* Filed in September 2017, from the discussion under patch 2/2:
https://bugzilla.tianocore.org/show_bug.cgi?id=714
In CONFIRMED status currently.
(The link to the original email discussion (in comment#0 of the BZ) no
longer works, because Intel has killed off the old <lists.01.org>
archive links meanwhile. But the <mail-archive.com> links should still
work.)
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77184): https://edk2.groups.io/g/devel/message/77184
Mute This Topic: https://groups.io/mt/83395028/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
