<div dir="ltr">Please read the Coverity report attached in BZ 4204 for more details on sign-extension issue.<div><br></div><div>The data types of <span style="color:rgb(31,35,40);font-family:ui-monospace,SFMono-Regular,"SF Mono",Menlo,Consolas,"Liberation Mono",monospace;font-size:12px;white-space:pre">logic_sector_size_hi </span>and <span style="color:rgb(31,35,40);font-family:ui-monospace,SFMono-Regular,"SF Mono",Menlo,Consolas,"Liberation Mono",monospace;font-size:12px;white-space:pre">logic_sector_size_lo </span>are UINT16 and Isn't the return type of <b>sizeof</b> already unsigned ? <br></div><div><br></div><div><div>Now I have no means to run Coverity and test further changes.</div><div>Anyway, my understanding back then was that the sign-extension was primarily happening because of the 16 bits left shift operation. I did test the patch for coverity warning resolution back in Jan'23 which went off.</div></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jun 5, 2023 at 4:28 AM Ard Biesheuvel <<a href="mailto:ardb@kernel.org">ardb@kernel.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, 2 Jun 2023 at 21:42, Ranbir Singh <<a href="mailto:rsingh@ventanamicro.com" target="_blank">rsingh@ventanamicro.com</a>> wrote:<br>
><br>
> From: Ranbir Singh <Ranbir.Singh3@Dell.com><br>
><br>
> Line number 1348 does contain a typecast with UINT32, but it is after<br>
> all the operations (16-bit left shift followed by OR'ing) are over.<br>
> To avoid any SIGN_EXTENSION, typecast the intermediate result after<br>
> 16-bit left shift operation immediately with UINT32.<br>
><br>
<br>
What is wrong with sign extension?<br>
<br>
> Cc: Hao A Wu <<a href="mailto:hao.a.wu@intel.com" target="_blank">hao.a.wu@intel.com</a>><br>
> Cc: Ray Ni <<a href="mailto:ray.ni@intel.com" target="_blank">ray.ni@intel.com</a>><br>
> REF: <a href="https://bugzilla.tianocore.org/show_bug.cgi?id=4204" rel="noreferrer" target="_blank">https://bugzilla.tianocore.org/show_bug.cgi?id=4204</a><br>
> Signed-off-by: Ranbir Singh <Ranbir.Singh3@Dell.com><br>
> ---<br>
> MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 2 +-<br>
> 1 file changed, 1 insertion(+), 1 deletion(-)<br>
><br>
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c<br>
> index 50406fe0270d..70c4ca27dc68 100644<br>
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c<br>
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c<br>
> @@ -1345,7 +1345,7 @@ AtaPassThruPassThru (<br>
> // Check logical block size<br>
> //<br>
> if ((IdentifyData->AtaData.phy_logic_sector_support & BIT12) != 0) {<br>
> - BlockSize = (UINT32)(((IdentifyData->AtaData.logic_sector_size_hi << 16) | IdentifyData->AtaData.logic_sector_size_lo) * sizeof (UINT16));<br>
> + BlockSize = (((UINT32)(IdentifyData->AtaData.logic_sector_size_hi << 16) | IdentifyData->AtaData.logic_sector_size_lo) * sizeof (UINT16));<br>
<br>
<br>
The outer parens are now redundant, which means you're assigning<br>
something to BlockSize whose type is based on the type of <something><br>
* sizeof(UINT16), which is unsigned long not unsigned int, so this<br>
will produce a truncation warning on some compilers.<br>
<br>
If you want to suppress the coverity warning without introducing new<br>
ones, better to cast the sizeof() to (UINT32).<br>
</blockquote></div>
<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/105719">View/Reply Online (#105719)</a> |
|
<a target="_blank" href="https://groups.io/mt/99293622/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>