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] -=-=-=-=-=-=-=-=-=-=-=-