hw/arm/xilinx_zynq.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
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
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
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
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
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
>
>
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
> >
> >
>
>
© 2016 - 2026 Red Hat, Inc.