On 01/15/2016 06:29 AM, Eric Blake wrote:
On 01/13/2016 05:51 AM, Cao jin wrote:
To catch the error msg. Also modify the caller
Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com>
---
hw/xen/xen-host-pci-device.c | 142 +++++++++++++++++++++----------------------
hw/xen/xen-host-pci-device.h | 5 +-
hw/xen/xen_pt.c | 13 ++--
3 files changed, 80 insertions(+), 80 deletions(-)
diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
index 351b61a..3e22de8 100644
--- a/hw/xen/xen-host-pci-device.c
+++ b/hw/xen/xen-host-pci-device.c
@@ -31,25 +31,20 @@
#define IORESOURCE_PREFETCH 0x00001000 /* No side effects */
#define IORESOURCE_MEM_64 0x00100000
-static int xen_host_pci_sysfs_path(const XenHostPCIDevice *d,
- const char *name, char *buf, ssize_t size)
+static void xen_host_pci_sysfs_path(const XenHostPCIDevice *d,
+ const char *name, char *buf, ssize_t size)
Changing xen_host_pci_sysfs_path() to return void, by assert()ing on
caller error, is not mentioned in the commit message; and if I were
doing the series, I probably would have done it as a separate commit.
Thanks for the suggestion, will split it out.
/* This size should be enough to read a long from a file */
#define XEN_HOST_PCI_GET_VALUE_BUFFER_SIZE 22
-static int xen_host_pci_get_value(XenHostPCIDevice *d, const char *name,
- unsigned int *pvalue, int base)
+static void xen_host_pci_get_value(XenHostPCIDevice *d, const char *name,
+ unsigned int *pvalue, int base, Error
**errp)
{
buf[rc] = 0;
- rc = qemu_strtoul(buf, &endptr, base, &value);
- if (!rc) {
- *pvalue = value;
+ rc = qemu_strtoul(buf, &endptr, base, (unsigned long *)pvalue);
Ouch. Casting unsigned int * to unsigned long * and then dereferencing
it is bogus (you end up having qemu_strtoul() write beyond bounds on
platforms where long is larger than int).
Yes, I considered this issue a little. Because the current condition is:
the value it want to get won`t exceed 4 byte (vendor/device ID, etc). So
I guess even if on x86_64(length of int != long), it won`t break things.
So, compared with following, which style do you prefer?
You'll need to revert this
part of the patch, and stick with *pvalue = value (and possibly even add
a bounds check that value <= UINT_MAX).
Otherwise looks okay.
--
Yours Sincerely,
Cao jin