[PATCH] hw/arm/xilinx_zynq: Use strcasecmp to parse boot-mode option values

Peter Maydell posted 1 patch 5 days, 23 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260327145012.907264-1-peter.maydell@linaro.org
Maintainers: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Alistair Francis <alistair@alistair23.me>, Peter Maydell <peter.maydell@linaro.org>
hw/arm/xilinx_zynq.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH] hw/arm/xilinx_zynq: Use strcasecmp to parse boot-mode option values
Posted by Peter Maydell 5 days, 23 hours ago
In zynq_set_boot_mode() where we parse the string the user has set
the boot-mode option to, we use strncasecmp(str, "qspi", 4) and so
on.  This is wrong, because it means that we will ignore any trailing
junk on the end of the option string, and handle
 -machine boot-mode=sdXYZZY
the same as
 -machine boot-mode=sd

In the documentation we say:
 Supported values are ``jtag``, ``sd``, ``qspi`` and ``nor``.
and that's obviously what we meant to implement.

The correct tool for this job is a simple strcasecmp operation.
Switch to that.

We use the g_ascii_strcasecmp() rather than plain strcasecmp()
because we're comparing ASCII strings here and don't want the
potentially locale-specific behaviour that strcasecmp() implies (and
we're trying to standardize on the glib function for this kind of
string comparison).

Fixes: 7df3747c92d13 ("hw/arm/xilinx_zynq: Add boot-mode property")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Kostiantyn: I sent this patch out while it was on my mind, but
don't worry about conflicts with your series -- assuming your
patchset goes in first I'll fix this one up later.
---
 hw/arm/xilinx_zynq.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index d43f36b718..9dcded9219 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -186,13 +186,13 @@ static void zynq_set_boot_mode(Object *obj, const char *str,
     ZynqMachineState *m = ZYNQ_MACHINE(obj);
     uint8_t mode = 0;
 
-    if (!strncasecmp(str, "qspi", 4)) {
+    if (!g_ascii_strcasecmp(str, "qspi")) {
         mode = 1;
-    } else if (!strncasecmp(str, "sd", 2)) {
+    } else if (!g_ascii_strcasecmp(str, "sd")) {
         mode = 5;
-    } else if (!strncasecmp(str, "nor", 3)) {
+    } else if (!g_ascii_strcasecmp(str, "nor")) {
         mode = 2;
-    } else if (!strncasecmp(str, "jtag", 4)) {
+    } else if (!g_ascii_strcasecmp(str, "jtag")) {
         mode = 0;
     } else {
         error_setg(errp, "%s boot mode not supported", str);
-- 
2.43.0
Re: [PATCH] hw/arm/xilinx_zynq: Use strcasecmp to parse boot-mode option values
Posted by Paolo Bonzini 3 days, 6 hours ago
On 3/27/26 15:50, Peter Maydell wrote:
> In zynq_set_boot_mode() where we parse the string the user has set
> the boot-mode option to, we use strncasecmp(str, "qspi", 4) and so
> on.  This is wrong, because it means that we will ignore any trailing
> junk on the end of the option string, and handle
>   -machine boot-mode=sdXYZZY
> the same as
>   -machine boot-mode=sd
> 
> In the documentation we say:
>   Supported values are ``jtag``, ``sd``, ``qspi`` and ``nor``.
> and that's obviously what we meant to implement.
> 
> The correct tool for this job is a simple strcasecmp operation.
> Switch to that.
> 
> We use the g_ascii_strcasecmp() rather than plain strcasecmp()
> because we're comparing ASCII strings here and don't want the
> potentially locale-specific behaviour that strcasecmp() implies (and
> we're trying to standardize on the glib function for this kind of
> string comparison).
> 
> Fixes: 7df3747c92d13 ("hw/arm/xilinx_zynq: Add boot-mode property")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

> Kostiantyn: I sent this patch out while it was on my mind, but
> don't worry about conflicts with your series -- assuming your
> patchset goes in first I'll fix this one up later.

No worries, go ahead since you can apply this even for 11.0; I'll take 
Kostiantyn's patches and handle the conflicts.

Paolo
Re: [PATCH] hw/arm/xilinx_zynq: Use strcasecmp to parse boot-mode option values
Posted by Peter Maydell 3 days, 4 hours ago
On Mon, 30 Mar 2026 at 08:35, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 3/27/26 15:50, Peter Maydell wrote:
> > In zynq_set_boot_mode() where we parse the string the user has set
> > the boot-mode option to, we use strncasecmp(str, "qspi", 4) and so
> > on.  This is wrong, because it means that we will ignore any trailing
> > junk on the end of the option string, and handle
> >   -machine boot-mode=sdXYZZY
> > the same as
> >   -machine boot-mode=sd
> >
> > In the documentation we say:
> >   Supported values are ``jtag``, ``sd``, ``qspi`` and ``nor``.
> > and that's obviously what we meant to implement.
> >
> > The correct tool for this job is a simple strcasecmp operation.
> > Switch to that.
> >
> > We use the g_ascii_strcasecmp() rather than plain strcasecmp()
> > because we're comparing ASCII strings here and don't want the
> > potentially locale-specific behaviour that strcasecmp() implies (and
> > we're trying to standardize on the glib function for this kind of
> > string comparison).
> >
> > Fixes: 7df3747c92d13 ("hw/arm/xilinx_zynq: Add boot-mode property")
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>
> > Kostiantyn: I sent this patch out while it was on my mind, but
> > don't worry about conflicts with your series -- assuming your
> > patchset goes in first I'll fix this one up later.
>
> No worries, go ahead since you can apply this even for 11.0; I'll take
> Kostiantyn's patches and handle the conflicts.

OK, great. I'm going to do a pullreq before rc2, so I'll put
this in there. Or if you're going to take Kostiantyn's series
and send it out for rc2 then feel free to put this patch in
there instead of the one that fixes up this file.

thanks
-- PMM
Re: [PATCH] hw/arm/xilinx_zynq: Use strcasecmp to parse boot-mode option values
Posted by Paolo Bonzini 3 days, 3 hours ago
On Mon, Mar 30, 2026 at 11:48 AM Peter Maydell <peter.maydell@linaro.org> wrote:
> OK, great. I'm going to do a pullreq before rc2, so I'll put
> this in there. Or if you're going to take Kostiantyn's series
> and send it out for rc2 then feel free to put this patch in
> there instead of the one that fixes up this file.

I wasn't planning to send his patches until after the release, since
alone they don't do much (they're not enough to allow the MSVC build
to complete).

Paolo
Re: [PATCH] hw/arm/xilinx_zynq: Use strcasecmp to parse boot-mode option values
Posted by Alistair Francis 3 days, 9 hours ago
On Sat, Mar 28, 2026 at 12:51 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In zynq_set_boot_mode() where we parse the string the user has set
> the boot-mode option to, we use strncasecmp(str, "qspi", 4) and so
> on.  This is wrong, because it means that we will ignore any trailing
> junk on the end of the option string, and handle
>  -machine boot-mode=sdXYZZY
> the same as
>  -machine boot-mode=sd
>
> In the documentation we say:
>  Supported values are ``jtag``, ``sd``, ``qspi`` and ``nor``.
> and that's obviously what we meant to implement.
>
> The correct tool for this job is a simple strcasecmp operation.
> Switch to that.
>
> We use the g_ascii_strcasecmp() rather than plain strcasecmp()
> because we're comparing ASCII strings here and don't want the
> potentially locale-specific behaviour that strcasecmp() implies (and
> we're trying to standardize on the glib function for this kind of
> string comparison).
>
> Fixes: 7df3747c92d13 ("hw/arm/xilinx_zynq: Add boot-mode property")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> Kostiantyn: I sent this patch out while it was on my mind, but
> don't worry about conflicts with your series -- assuming your
> patchset goes in first I'll fix this one up later.
> ---
>  hw/arm/xilinx_zynq.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index d43f36b718..9dcded9219 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -186,13 +186,13 @@ static void zynq_set_boot_mode(Object *obj, const char *str,
>      ZynqMachineState *m = ZYNQ_MACHINE(obj);
>      uint8_t mode = 0;
>
> -    if (!strncasecmp(str, "qspi", 4)) {
> +    if (!g_ascii_strcasecmp(str, "qspi")) {
>          mode = 1;
> -    } else if (!strncasecmp(str, "sd", 2)) {
> +    } else if (!g_ascii_strcasecmp(str, "sd")) {
>          mode = 5;
> -    } else if (!strncasecmp(str, "nor", 3)) {
> +    } else if (!g_ascii_strcasecmp(str, "nor")) {
>          mode = 2;
> -    } else if (!strncasecmp(str, "jtag", 4)) {
> +    } else if (!g_ascii_strcasecmp(str, "jtag")) {
>          mode = 0;
>      } else {
>          error_setg(errp, "%s boot mode not supported", str);
> --
> 2.43.0
>
>
Re: [PATCH] hw/arm/xilinx_zynq: Use strcasecmp to parse boot-mode option values
Posted by Kostiantyn Kostiuk 3 days, 7 hours ago
Reviewed-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>

On Mon, Mar 30, 2026 at 7:32 AM Alistair Francis <alistair23@gmail.com>
wrote:

> On Sat, Mar 28, 2026 at 12:51 AM Peter Maydell <peter.maydell@linaro.org>
> wrote:
> >
> > In zynq_set_boot_mode() where we parse the string the user has set
> > the boot-mode option to, we use strncasecmp(str, "qspi", 4) and so
> > on.  This is wrong, because it means that we will ignore any trailing
> > junk on the end of the option string, and handle
> >  -machine boot-mode=sdXYZZY
> > the same as
> >  -machine boot-mode=sd
> >
> > In the documentation we say:
> >  Supported values are ``jtag``, ``sd``, ``qspi`` and ``nor``.
> > and that's obviously what we meant to implement.
> >
> > The correct tool for this job is a simple strcasecmp operation.
> > Switch to that.
> >
> > We use the g_ascii_strcasecmp() rather than plain strcasecmp()
> > because we're comparing ASCII strings here and don't want the
> > potentially locale-specific behaviour that strcasecmp() implies (and
> > we're trying to standardize on the glib function for this kind of
> > string comparison).
> >
> > Fixes: 7df3747c92d13 ("hw/arm/xilinx_zynq: Add boot-mode property")
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
> Alistair
>
> > ---
> > Kostiantyn: I sent this patch out while it was on my mind, but
> > don't worry about conflicts with your series -- assuming your
> > patchset goes in first I'll fix this one up later.
> > ---
> >  hw/arm/xilinx_zynq.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> > index d43f36b718..9dcded9219 100644
> > --- a/hw/arm/xilinx_zynq.c
> > +++ b/hw/arm/xilinx_zynq.c
> > @@ -186,13 +186,13 @@ static void zynq_set_boot_mode(Object *obj, const
> char *str,
> >      ZynqMachineState *m = ZYNQ_MACHINE(obj);
> >      uint8_t mode = 0;
> >
> > -    if (!strncasecmp(str, "qspi", 4)) {
> > +    if (!g_ascii_strcasecmp(str, "qspi")) {
> >          mode = 1;
> > -    } else if (!strncasecmp(str, "sd", 2)) {
> > +    } else if (!g_ascii_strcasecmp(str, "sd")) {
> >          mode = 5;
> > -    } else if (!strncasecmp(str, "nor", 3)) {
> > +    } else if (!g_ascii_strcasecmp(str, "nor")) {
> >          mode = 2;
> > -    } else if (!strncasecmp(str, "jtag", 4)) {
> > +    } else if (!g_ascii_strcasecmp(str, "jtag")) {
> >          mode = 0;
> >      } else {
> >          error_setg(errp, "%s boot mode not supported", str);
> > --
> > 2.43.0
> >
> >
>
>