<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><div dir="ltr"></div><div dir="ltr">As discussion got stuck,</div><div dir="ltr"><br></div><div dir="ltr">Reviewed-by: Marvin Häuser <mhaeuser@posteo.de></div><div dir="ltr"><br><blockquote type="cite">On 27. Jan 2023, at 15:36, Marvin Häuser <mhaeuser@posteo.de> wrote:<br><br></blockquote></div><blockquote type="cite"><div dir="ltr"><meta http-equiv="content-type" content="text/html; charset=utf-8">On 27. Jan 2023, at 15:33, Pedro Falcato <pedro.falcato@gmail.com> wrote:<br><div><blockquote type="cite"><br class="Apple-interchange-newline"><div><meta charset="UTF-8"><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;">On Fri, Jan 27, 2023 at 9:29 AM Savva Mitrofanov <</span><a href="mailto:savvamtr@gmail.com" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;">savvamtr@gmail.com</a><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;">> wrote:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;"><br>Accessing array using index of uint64 type makes MSVC compiler to<br>include `__allmul` function in NOOPT which is not referenced in IA32.<br>So we null-terminates string using ReadSize, which should be equal to<br>SymlinkSizeTmp after correct reading. Also adds missing MultU64x32<br>in Ext4Read.<br><br>Cc: Marvin Häuser <mhaeuser@posteo.de><br>Cc: Pedro Falcato <pedro.falcato@gmail.com><br>Cc: Vitaly Cheptsov <vit9696@protonmail.com><br>Fixes: 7c46116b0e18 ("Ext4Pkg: Add ext2/3 support")<br>Fixes: e81432fbacb7 ("Ext4Pkg: Add symbolic links support")<br>Signed-off-by: Savva Mitrofanov <savvamtr@gmail.com><br>---<br>Features/Ext4Pkg/Ext4Dxe/Inode.c | 4 ++--<br>Features/Ext4Pkg/Ext4Dxe/Symlink.c | 12 ++++++------<br>2 files changed, 8 insertions(+), 8 deletions(-)<br><br>diff --git a/Features/Ext4Pkg/Ext4Dxe/Inode.c b/Features/Ext4Pkg/Ext4Dxe/Inode.c<br>index 90e3eb88f523..8db051d3c444 100644<br>--- a/Features/Ext4Pkg/Ext4Dxe/Inode.c<br>+++ b/Features/Ext4Pkg/Ext4Dxe/Inode.c<br>@@ -152,7 +152,7 @@ Ext4Read (<br> } else {<br> // Uninitialized extents behave exactly the same as file holes, except they have<br> // blocks already allocated to them.<br>- HoleLen = (Ext4GetExtentLength (&Extent) * Partition->BlockSize) - HoleOff;<br>+ HoleLen = MultU64x32 (Ext4GetExtentLength (&Extent), Partition->BlockSize) - HoleOff;<br> }<br><br> WasRead = HoleLen > RemainingRead ? RemainingRead : (UINTN)HoleLen;<br>@@ -166,7 +166,7 @@ Ext4Read (<br> Partition->BlockSize<br> );<br> ExtentLengthBytes = Extent.ee_len * Partition->BlockSize;<br>- ExtentLogicalBytes = (UINT64)Extent.ee_block * Partition->BlockSize;<br>+ ExtentLogicalBytes = MultU64x32 ((UINT64)Extent.ee_block, Partition->BlockSize);<br> ExtentOffset = CurrentSeek - ExtentLogicalBytes;<br> ExtentMayRead = (UINTN)(ExtentLengthBytes - ExtentOffset);<br><br>diff --git a/Features/Ext4Pkg/Ext4Dxe/Symlink.c b/Features/Ext4Pkg/Ext4Dxe/Symlink.c<br>index 19b357ac6ba0..8b1511a38b55 100644<br>--- a/Features/Ext4Pkg/Ext4Dxe/Symlink.c<br>+++ b/Features/Ext4Pkg/Ext4Dxe/Symlink.c<br>@@ -1,7 +1,7 @@<br>/** @file<br> Symbolic links routines<br><br>- Copyright (c) 2022 Savva Mitrofanov All rights reserved.<br>+ Copyright (c) 2022-2023 Savva Mitrofanov All rights reserved.<br> SPDX-License-Identifier: BSD-2-Clause-Patent<br>**/<br><br>@@ -155,11 +155,6 @@ Ext4ReadSlowSymlink (<br> return Status;<br> }<br><br>- //<br>- // Add null-terminator<br>- //<br>- SymlinkTmp[SymlinkSizeTmp] = '\0';<br>-<br> if (SymlinkSizeTmp != ReadSize) {<br> DEBUG ((<br> DEBUG_FS,<br>@@ -168,6 +163,11 @@ Ext4ReadSlowSymlink (<br> return EFI_VOLUME_CORRUPTED;<br> }<br><br>+ //<br>+ // Add null-terminator<br>+ //<br></blockquote><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;">Sidenote: please don't use this comment style, I know it's prevalent</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;">in EDK2 and EDK2 platforms but Ext4Pkg just does:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;">// Comment</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;">Also, why are you bringing this null-termination down here?</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"></div></blockquote><div><br></div><div>I don't think there's a strong functional reason, but it adheres to the principle of not touching malformed data if at all possible. Not sure it needs to be part of this particular commit, but why not, it causes less noise this way.</div><br><blockquote type="cite"><div><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;">+ SymlinkTmp[ReadSize] = '\0';<br>+<br> *AsciiSymlinkSize = SymlinkAllocateSize;<br> *AsciiSymlink = SymlinkTmp;<br><br>--<br>2.39.0<br><br></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;">--<span class="Apple-converted-space"> </span></span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;">Pedro</span></div></blockquote></div><br></div></blockquote></body></html>
<div width="1" style="color:white;clear:both">_._,_._,_</div>
<hr>
Groups.io Links:<p>
You receive all messages sent to this group.
<p>
<a target="_blank" href="https://edk2.groups.io/g/devel/message/99294">View/Reply Online (#99294)</a> |
|
<a target="_blank" href="https://groups.io/mt/96562700/1813853">Mute This Topic</a>
| <a href="https://edk2.groups.io/g/devel/post">New Topic</a><br>
<a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> |
<a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> |
<a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a>
[edk2-devel-archive@redhat.com]<br>
<div width="1" style="color:white;clear:both">_._,_._,_</div>