This reverts commit bccb20c49df1bd683248a366021973901c11982f as it
introduced a regression blocking bus addresses > 0x1f or higher.
Legal bus numbers go up to 0xff.
Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com>
Fixes: bccb20c49df ("Use qemu_strtoul() in set_pci_host_devaddr()")
Reported-by: Klaus Herman <kherman@inbox.lv>
Reported-by: Geoffrey McRae <geoff@hostfission.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
checkpatch will complain since it does not like strtoul but
we had it for years so should be ok for 5.2, right?
hw/core/qdev-properties-system.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index b81a4e8d14..9d80a07d26 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -858,7 +858,7 @@ static void set_pci_host_devaddr(Object *obj, Visitor *v, const char *name,
Property *prop = opaque;
PCIHostDeviceAddress *addr = qdev_get_prop_ptr(dev, prop);
char *str, *p;
- const char *e;
+ char *e;
unsigned long val;
unsigned long dom = 0, bus = 0;
unsigned int slot = 0, func = 0;
@@ -873,23 +873,23 @@ static void set_pci_host_devaddr(Object *obj, Visitor *v, const char *name,
}
p = str;
- if (qemu_strtoul(p, &e, 16, &val) < 0 || val > 0xffff || e == p) {
- goto inval;
- }
- if (*e != ':') {
+ val = strtoul(p, &e, 16);
+ if (e == p || *e != ':') {
goto inval;
}
bus = val;
- p = (char *)e + 1;
- if (qemu_strtoul(p, &e, 16, &val) < 0 || val > 0x1f || e == p) {
+ p = e + 1;
+ val = strtoul(p, &e, 16);
+ if (e == p) {
goto inval;
}
if (*e == ':') {
dom = bus;
bus = val;
- p = (char *)e + 1;
- if (qemu_strtoul(p, &e, 16, &val) < 0 || val > 0x1f || e == p) {
+ p = e + 1;
+ val = strtoul(p, &e, 16);
+ if (e == p) {
goto inval;
}
}
@@ -898,13 +898,14 @@ static void set_pci_host_devaddr(Object *obj, Visitor *v, const char *name,
if (*e != '.') {
goto inval;
}
- p = (char *)e + 1;
- if (qemu_strtoul(p, &e, 10, &val) < 0 || val > 7 || e == p) {
+ p = e + 1;
+ val = strtoul(p, &e, 10);
+ if (e == p) {
goto inval;
}
func = val;
- if (bus > 0xff) {
+ if (dom > 0xffff || bus > 0xff || slot > 0x1f || func > 7) {
goto inval;
}
--
MST
Patchew URL: https://patchew.org/QEMU/20201120130409.956956-1-mst@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20201120130409.956956-1-mst@redhat.com Subject: [PATCH for-5.2] Revert "hw/core/qdev-properties: Use qemu_strtoul() in set_pci_host_devaddr()" === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu - [tag update] patchew/20201120073149.99079-1-pbonzini@redhat.com -> patchew/20201120073149.99079-1-pbonzini@redhat.com * [new tag] patchew/20201120130409.956956-1-mst@redhat.com -> patchew/20201120130409.956956-1-mst@redhat.com Switched to a new branch 'test' 1d9f6bd Revert "hw/core/qdev-properties: Use qemu_strtoul() in set_pci_host_devaddr()" === OUTPUT BEGIN === ERROR: consider using qemu_strtoul in preference to strtoul #39: FILE: hw/core/qdev-properties-system.c:876: + val = strtoul(p, &e, 16); ERROR: consider using qemu_strtoul in preference to strtoul #48: FILE: hw/core/qdev-properties-system.c:883: + val = strtoul(p, &e, 16); ERROR: consider using qemu_strtoul in preference to strtoul #58: FILE: hw/core/qdev-properties-system.c:891: + val = strtoul(p, &e, 16); ERROR: consider using qemu_strtoul in preference to strtoul #70: FILE: hw/core/qdev-properties-system.c:902: + val = strtoul(p, &e, 10); total: 4 errors, 0 warnings, 56 lines checked Commit 1d9f6bd1223f (Revert "hw/core/qdev-properties: Use qemu_strtoul() in set_pci_host_devaddr()") has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20201120130409.956956-1-mst@redhat.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
On 20/11/20 14:04, Michael S. Tsirkin wrote: > This reverts commit bccb20c49df1bd683248a366021973901c11982f as it > introduced a regression blocking bus addresses > 0x1f or higher. > Legal bus numbers go up to 0xff. > > Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com> > Fixes: bccb20c49df ("Use qemu_strtoul() in set_pci_host_devaddr()") > Reported-by: Klaus Herman <kherman@inbox.lv> > Reported-by: Geoffrey McRae <geoff@hostfission.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > > checkpatch will complain since it does not like strtoul but > we had it for years so should be ok for 5.2, right? Yes, of course. Paolo > hw/core/qdev-properties-system.c | 25 +++++++++++++------------ > 1 file changed, 13 insertions(+), 12 deletions(-) > > diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c > index b81a4e8d14..9d80a07d26 100644 > --- a/hw/core/qdev-properties-system.c > +++ b/hw/core/qdev-properties-system.c > @@ -858,7 +858,7 @@ static void set_pci_host_devaddr(Object *obj, Visitor *v, const char *name, > Property *prop = opaque; > PCIHostDeviceAddress *addr = qdev_get_prop_ptr(dev, prop); > char *str, *p; > - const char *e; > + char *e; > unsigned long val; > unsigned long dom = 0, bus = 0; > unsigned int slot = 0, func = 0; > @@ -873,23 +873,23 @@ static void set_pci_host_devaddr(Object *obj, Visitor *v, const char *name, > } > > p = str; > - if (qemu_strtoul(p, &e, 16, &val) < 0 || val > 0xffff || e == p) { > - goto inval; > - } > - if (*e != ':') { > + val = strtoul(p, &e, 16); > + if (e == p || *e != ':') { > goto inval; > } > bus = val; > > - p = (char *)e + 1; > - if (qemu_strtoul(p, &e, 16, &val) < 0 || val > 0x1f || e == p) { > + p = e + 1; > + val = strtoul(p, &e, 16); > + if (e == p) { > goto inval; > } > if (*e == ':') { > dom = bus; > bus = val; > - p = (char *)e + 1; > - if (qemu_strtoul(p, &e, 16, &val) < 0 || val > 0x1f || e == p) { > + p = e + 1; > + val = strtoul(p, &e, 16); > + if (e == p) { > goto inval; > } > } > @@ -898,13 +898,14 @@ static void set_pci_host_devaddr(Object *obj, Visitor *v, const char *name, > if (*e != '.') { > goto inval; > } > - p = (char *)e + 1; > - if (qemu_strtoul(p, &e, 10, &val) < 0 || val > 7 || e == p) { > + p = e + 1; > + val = strtoul(p, &e, 10); > + if (e == p) { > goto inval; > } > func = val; > > - if (bus > 0xff) { > + if (dom > 0xffff || bus > 0xff || slot > 0x1f || func > 7) { > goto inval; > } > >
On 11/20/20 2:10 PM, Paolo Bonzini wrote: > On 20/11/20 14:04, Michael S. Tsirkin wrote: >> This reverts commit bccb20c49df1bd683248a366021973901c11982f as it >> introduced a regression blocking bus addresses > 0x1f or higher. >> Legal bus numbers go up to 0xff. >> >> Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com> >> Fixes: bccb20c49df ("Use qemu_strtoul() in set_pci_host_devaddr()") >> Reported-by: Klaus Herman <kherman@inbox.lv> >> Reported-by: Geoffrey McRae <geoff@hostfission.com> >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> >> --- >> >> checkpatch will complain since it does not like strtoul but >> we had it for years so should be ok for 5.2, right? > > Yes, of course. > > Paolo Which tree is going to merge this patch?
On Tue, Nov 24, 2020 at 03:13:14PM +0100, Philippe Mathieu-Daudé wrote: > On 11/20/20 2:10 PM, Paolo Bonzini wrote: > > On 20/11/20 14:04, Michael S. Tsirkin wrote: > >> This reverts commit bccb20c49df1bd683248a366021973901c11982f as it > >> introduced a regression blocking bus addresses > 0x1f or higher. > >> Legal bus numbers go up to 0xff. > >> > >> Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com> > >> Fixes: bccb20c49df ("Use qemu_strtoul() in set_pci_host_devaddr()") > >> Reported-by: Klaus Herman <kherman@inbox.lv> > >> Reported-by: Geoffrey McRae <geoff@hostfission.com> > >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > >> --- > >> > >> checkpatch will complain since it does not like strtoul but > >> we had it for years so should be ok for 5.2, right? > > > > Yes, of course. > > > > Paolo > > Which tree is going to merge this patch? Sorry, I was expecting Michael to merge it, as it's PCI-specific but it looks like people were waiting for me to merge it. I'll merge it and submit a pull request ASAP. -- Eduardo
On 11/24/20 4:06 PM, Eduardo Habkost wrote: > On Tue, Nov 24, 2020 at 03:13:14PM +0100, Philippe Mathieu-Daudé wrote: >> On 11/20/20 2:10 PM, Paolo Bonzini wrote: >>> On 20/11/20 14:04, Michael S. Tsirkin wrote: >>>> This reverts commit bccb20c49df1bd683248a366021973901c11982f as it >>>> introduced a regression blocking bus addresses > 0x1f or higher. >>>> Legal bus numbers go up to 0xff. >>>> >>>> Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com> >>>> Fixes: bccb20c49df ("Use qemu_strtoul() in set_pci_host_devaddr()") >>>> Reported-by: Klaus Herman <kherman@inbox.lv> >>>> Reported-by: Geoffrey McRae <geoff@hostfission.com> >>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> >>>> --- >>>> >>>> checkpatch will complain since it does not like strtoul but >>>> we had it for years so should be ok for 5.2, right? >>> >>> Yes, of course. >>> >>> Paolo >> >> Which tree is going to merge this patch? > > Sorry, I was expecting Michael to merge it, as it's PCI-specific > but it looks like people were waiting for me to merge it. > > I'll merge it and submit a pull request ASAP. Thanks!
On 11/20/20 2:04 PM, Michael S. Tsirkin wrote: > This reverts commit bccb20c49df1bd683248a366021973901c11982f as it > introduced a regression blocking bus addresses > 0x1f or higher. > Legal bus numbers go up to 0xff. > > Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com> > Fixes: bccb20c49df ("Use qemu_strtoul() in set_pci_host_devaddr()") > Reported-by: Klaus Herman <kherman@inbox.lv> > Reported-by: Geoffrey McRae <geoff@hostfission.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > > checkpatch will complain since it does not like strtoul but > we had it for years so should be ok for 5.2, right? > > hw/core/qdev-properties-system.c | 25 +++++++++++++------------ > 1 file changed, 13 insertions(+), 12 deletions(-) Thanks! Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
© 2016 - 2024 Red Hat, Inc.