From: "Laszlo Ersek" <lersek@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>, devel@edk2.groups.io
Cc: oliver@redhat.com
Subject: Re: [edk2-devel] [PATCH v2 1/6] OvmfPkg/VirtNorFlashDxe: add casts to UINTN and UINT32
Date: Mon, 15 Jan 2024 20:20:32 +0100 [thread overview]
Message-ID: <55e5b50e-b635-9cef-7d24-56bfce40d746@redhat.com> (raw)
In-Reply-To: <20240115155948.136499-2-kraxel@redhat.com>
On 1/15/24 16:59, Gerd Hoffmann wrote:
> This is needed to avoid bit operations being applied to signed integers.
>
> Suggested-by: László Érsek <lersek@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> OvmfPkg/VirtNorFlashDxe/VirtNorFlash.h | 2 +-
> OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.h b/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.h
> index b7f5d208b236..455eafacc2cf 100644
> --- a/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.h
> +++ b/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.h
> @@ -61,7 +61,7 @@
> #define P30_MAX_BUFFER_SIZE_IN_BYTES ((UINTN)128)
> #define P30_MAX_BUFFER_SIZE_IN_WORDS (P30_MAX_BUFFER_SIZE_IN_BYTES/((UINTN)4))
> #define MAX_BUFFERED_PROG_ITERATIONS 10000000
> -#define BOUNDARY_OF_32_WORDS 0x7F
> +#define BOUNDARY_OF_32_WORDS ((UINTN)0x7F)
>
> // CFI Addresses
> #define P30_CFI_ADDR_QUERY_UNIQUE_QRY 0x10
I've made an effort to audit all current (= pre-patch) uses of
BOUNDARY_OF_32_WORDS: the change looks safe.
> diff --git a/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c b/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c
> index 1afd60ce66eb..7f4743b00399 100644
> --- a/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c
> +++ b/OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c
> @@ -581,7 +581,7 @@ NorFlashWriteSingleBlock (
> // contents, while checking whether the old version had any bits cleared
> // that we want to set. In that case, we will need to erase the block first.
> for (CurOffset = 0; CurOffset < *NumBytes; CurOffset++) {
> - if (~OrigData[CurOffset] & Buffer[CurOffset]) {
> + if (~(UINT32)OrigData[CurOffset] & (UINT32)Buffer[CurOffset]) {
> goto DoErase;
> }
>
The explicit cast for the RHS is not strictly necessary (the same would
happen as a consequence of the cast being added to the LHS, through the
usual arithmetic conversions), *but* it definitely doesn't hurt, and
arguably improves readability.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
(I'll probably look at the rest of the patches tomorrow.)
Thanks!
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113846): https://edk2.groups.io/g/devel/message/113846
Mute This Topic: https://groups.io/mt/103741662/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
next prev parent reply other threads:[~2024-01-16 6:27 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-15 15:59 [edk2-devel] [PATCH v2 0/6] OvmfPkg/VirtNorFlashDxe: fix corruption + misc small improvements Gerd Hoffmann
2024-01-15 15:59 ` [edk2-devel] [PATCH v2 1/6] OvmfPkg/VirtNorFlashDxe: add casts to UINTN and UINT32 Gerd Hoffmann
2024-01-15 19:20 ` Laszlo Ersek [this message]
2024-01-15 15:59 ` [edk2-devel] [PATCH v2 2/6] OvmfPkg/VirtNorFlashDxe: clarify block write logic & fix shadowbuffer reads Gerd Hoffmann
2024-01-16 12:48 ` Laszlo Ersek
2024-01-15 15:59 ` [edk2-devel] [PATCH v2 3/6] OvmfPkg/VirtNorFlashDxe: add a loop for NorFlashWriteBuffer calls Gerd Hoffmann
2024-01-16 12:56 ` Laszlo Ersek
2024-01-15 15:59 ` [edk2-devel] [PATCH v2 4/6] OvmfPkg/VirtNorFlashDxe: allow larger writes without block erase Gerd Hoffmann
2024-01-16 12:59 ` Laszlo Ersek
2024-01-15 15:59 ` [edk2-devel] [PATCH v2 5/6] OvmfPkg/VirtNorFlashDxe: ValidateFvHeader: unwritten state is EOL too Gerd Hoffmann
2024-01-16 13:01 ` Laszlo Ersek
2024-01-15 15:59 ` [edk2-devel] [PATCH v2 6/6] OvmfPkg/VirtNorFlashDxe: move DoErase code block into new function Gerd Hoffmann
2024-01-16 13:44 ` Laszlo Ersek
2024-01-16 15:29 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55e5b50e-b635-9cef-7d24-56bfce40d746@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox