Re: [PATCH 08/13] conf: Introduce parser, formatter for uid and fid

[Laine Stump <laine@xxxxxxxxx>]

On 05/25/2018 07:05 AM, Pino Toscano wrote:

On Thursday, 24 May 2018 14:24:33 CEST Xiao Feng Ren wrote:

From: Yi Min Zhao <zyimin@xxxxxxxxxxxxx>

This patch introduces new XML parser/formatter functions. Uid is
16-bit and non-zero. Fid is 32-bit. They are added as two new
attributes of PCI address, and parsed/formatted along with PCI
address parser/formatter.

Signed-off-by: Yi Min Zhao <zyimin@xxxxxxxxxxxxx>
Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Stefan Zimmermann <stzi@xxxxxxxxxxxxx>
Reviewed-by: Bjoern Walk <bwalk@xxxxxxxxxxxxxxxxxx>
---
 docs/schemas/basictypes.rng   | 28 ++++++++++++++++
 docs/schemas/domaincommon.rng |  1 +
 src/conf/device_conf.c        | 74 +++++++++++++++++++++++++++++++++++++++++++
 src/conf/domain_addr.c        |  3 ++
 src/conf/domain_conf.c        |  4 +++
 src/util/virpci.h             |  3 ++
 6 files changed, 113 insertions(+)

diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 1a18cd31b1..8050a3ebc4 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -111,6 +111,34 @@
       </attribute>
     </optional>
   </define>
+  <define name="zpciaddress">
+    <optional>
+      <attribute name="uid">
+        <choice>
+          <data type="string">
+            <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param>
+          </data>
+          <data type="unsignedInt">
+            <param name="minInclusive">1</param>
+            <param name="maxInclusive">65535</param>
+          </data>
+        </choice>
+      </attribute>

This seems to be the "uint16" type already.

+    </optional>
+    <optional>
+      <attribute name="fid">
+        <choice>
+          <data type="string">
+            <param name="pattern">(0x)?[0-9a-fA-F]{1,8}</param>
+          </data>
+          <data type="unsignedLong">
+            <param name="minInclusive">0</param>
+            <param name="maxInclusive">4294967295</param>
+          </data>
+        </choice>
+      </attribute>

This could be a new "uint32" type, changing the 0x prefix as
non-optional (otherwise the value "10" can be both valid as decimal
and hexadeciaml).

My personal opinion - if numbers without a leading 0x are consistently not interpreted as hexadecimal, then there is no ambiguity and no confusion. If there's a leading 0x, then it is hexadecimal. No leading 0x, it's decimal. Period.

@@ -57,6 +125,8 @@ void
 virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
 {
     VIR_FREE(info->alias);
+    if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)
+        VIR_FREE(info->addr.pci.zpci);

VIR_FREE should be safe to use on a NULL pointer, so just call it
directly without checking the type.

But this code isn't checking for a NULL pointer. It's checking to see which member of the union is being used - there may be a different address type that uses the same region of memory as something that isn't a pointer, and calling VIR_FREE would lead to dereferencing a bad pointer.

     memset(&info->addr, 0, sizeof(info->addr));
     info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
     VIR_FREE(info->romfile);
@@ -187,6 +257,7 @@ int virPCIDeviceAddressIsValid(virPCIDeviceAddressPtr addr,
                              "one of domain, bus, or slot must be > 0"));
         return 0;
     }
+
     return 1;
 }

Extra change.

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list