As Pedro agreed, forgot to add Reviewed-by: Marvin Häuser <mhaeu...@posteo.de>
@Pedro feel free to change to UINT8 on push nevertheless. :) > On 27. Jan 2023, at 15:14, Marvin Häuser <mhaeu...@posteo.de> wrote: > > > >> On 27. Jan 2023, at 15:09, Pedro Falcato <pedro.falc...@gmail.com> wrote: >> >>> On Fri, Jan 27, 2023 at 10:04 AM Marvin Häuser <mhaeu...@posteo.de> wrote: >>> >>> On 27. Jan 2023, at 10:29, Savva Mitrofanov <savva...@gmail.com> wrote: >>>> >>>> The directory entry name conventions forbid having null-terminator >>>> symbols in its body and can lead to undefined behavior conditions >>>> and crashes >>>> >>>> Cc: Marvin Häuser <mhaeu...@posteo.de> >>>> Cc: Pedro Falcato <pedro.falc...@gmail.com> >>>> Cc: Vitaly Cheptsov <vit9...@protonmail.com> >>>> Fixes: 89b2bb0db263 ("Ext4Pkg: Fix and clarify handling regarding non-utf8 >>>> dir entries") >>>> Signed-off-by: Savva Mitrofanov <savva...@gmail.com> >>>> --- >>>> Features/Ext4Pkg/Ext4Dxe/Directory.c | 9 ++++++++- >>>> 1 file changed, 8 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/Features/Ext4Pkg/Ext4Dxe/Directory.c >>>> b/Features/Ext4Pkg/Ext4Dxe/Directory.c >>>> index 0753a20b5377..465749c9b51d 100644 >>>> --- a/Features/Ext4Pkg/Ext4Dxe/Directory.c >>>> +++ b/Features/Ext4Pkg/Ext4Dxe/Directory.c >>>> @@ -28,9 +28,16 @@ Ext4GetUcs2DirentName ( >>>> { >>>> CHAR8 Utf8NameBuf[EXT4_NAME_MAX + 1]; >>>> UINT16 *Str; >>>> + UINTN Index; >>> >>> I *really* do not like UINTN in code that does not deal with buffer >>> addresses and sizes. I'd change it to UINT8, but I'll leave it up to Pedro. >> Considering this is a size (length of name) and it gets explicitly > > Well, being a "size" is not sufficient, I explicitly referred to buffer > sizes. Say, you get an arbitrarily long buffer from a caller, its size > parameter should probably be UINTN, to be able to optimally leverage the > platform memory and be flexible. This is not a buffer size, this is a bounded > index of an architecture-agnostic data structure. I simply do not want > behaviour that depends on the host architecture in architecture-agnostic code > for no reason. We actually had multiple prior examples of bugs due to this, > though this is obviously fine (thus not demanding it's changed). > >> bounded by name_len (a UINT8) I'm okay with it. But if for some reason >> you need to submit a v4, please change. >> >> Reviewed-by: Pedro Falcato <pedro.falc...@gmail.com> >> >> -- >> Pedro > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99297): https://edk2.groups.io/g/devel/message/99297 Mute This Topic: https://groups.io/mt/96562701/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-