12:25 p.m.
virDomainChrGetDomainPtrs() required 4 levels of pointers (taking
a parameter that will be used as an output variable to return the
address of another variable that contains an array of pointers).
This is rather complex to reason about, especially when outside
of the domain_conf file, no other caller should be modifying
the resulting array of pointers directly. Changing the public
signature gives something is easier to reason with, and actually
make const-correct; which is important as it was the only function
that was blocking virDomainDeviceDefCopy from treating its source
as const.
* src/conf/domain_conf.h (virDomainChrGetDomainPtrs): Use simpler
types, and make const-correct for external users.
* src/conf/domain_conf.c (virDomainChrGetDomainPtrs): Split...
(virDomainChrGetDomainPtrsInternal): ...into an internal version
that lets us modify terms, vs. external form that is read-only.
(virDomainDeviceDefPostParseInternal, virDomainChrFind)
(virDomainChrInsert): Adjust callers.
* src/qemu/qemu_command.c (qemuGetNextChrDevIndex): Adjust caller.
(qemuDomainDeviceAliasIndex): Make const-correct.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/conf/domain_conf.c | 70 ++++++++++++++++++++++++++++++++++---------------
src/conf/domain_conf.h | 9 ++++---
src/qemu/qemu_command.c | 14 +++++-----
3 files changed, 61 insertions(+), 32 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0d63845..8e1e644 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2807,10 +2807,10 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev,
{
if (dev->type == VIR_DOMAIN_DEVICE_CHR) {
virDomainChrDefPtr chr = dev->data.chr;
- virDomainChrDefPtr **arrPtr;
- size_t i, *cnt;
+ const virDomainChrDef **arrPtr;
+ size_t i, cnt;
- virDomainChrGetDomainPtrs(def, chr, &arrPtr, &cnt);
+ virDomainChrGetDomainPtrs(def, chr->deviceType, &arrPtr, &cnt);
if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE)
@@ -2822,9 +2822,9 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev,
chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE)) {
int maxport = -1;
- for (i = 0; i < *cnt; i++) {
- if ((*arrPtr)[i]->target.port > maxport)
- maxport = (*arrPtr)[i]->target.port;
+ for (i = 0; i < cnt; i++) {
+ if (arrPtr[i]->target.port > maxport)
+ maxport = arrPtr[i]->target.port;
}
chr->target.port = maxport + 1;
@@ -2834,8 +2834,8 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev,
chr->info.addr.vioserial.port == 0) {
int maxport = 0;
- for (i = 0; i < *cnt; i++) {
- virDomainChrDefPtr thischr = (*arrPtr)[i];
+ for (i = 0; i < cnt; i++) {
+ const virDomainChrDef *thischr = arrPtr[i];
if (thischr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL
&&
thischr->info.addr.vioserial.controller ==
chr->info.addr.vioserial.controller &&
thischr->info.addr.vioserial.bus ==
chr->info.addr.vioserial.bus &&
@@ -10278,26 +10278,31 @@ virDomainChrDefPtr
virDomainChrFind(virDomainDefPtr def,
virDomainChrDefPtr target)
{
- virDomainChrDefPtr chr, **arrPtr;
- size_t i, *cntPtr;
+ virDomainChrDefPtr chr;
+ const virDomainChrDef **arrPtr;
+ size_t i, cnt;
- virDomainChrGetDomainPtrs(def, target, &arrPtr, &cntPtr);
+ virDomainChrGetDomainPtrs(def, target->deviceType, &arrPtr, &cnt);
- for (i = 0; i < *cntPtr; i++) {
- chr = (*arrPtr)[i];
+ for (i = 0; i < cnt; i++) {
+ /* Cast away const */
+ chr = (virDomainChrDefPtr) arrPtr[i];
if (virDomainChrEquals(chr, target))
return chr;
}
return NULL;
}
-void
-virDomainChrGetDomainPtrs(virDomainDefPtr vmdef,
- virDomainChrDefPtr chr,
- virDomainChrDefPtr ***arrPtr,
- size_t **cntPtr)
+
+/* Return the address within vmdef to be modified when working with a
+ * chrdefptr of the given type. */
+static void
+virDomainChrGetDomainPtrsInternal(virDomainDefPtr vmdef,
+ enum virDomainChrDeviceType type,
+ virDomainChrDefPtr ***arrPtr,
+ size_t **cntPtr)
{
- switch ((enum virDomainChrDeviceType) chr->deviceType) {
+ switch (type) {
case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL:
*arrPtr = &vmdef->parallels;
*cntPtr = &vmdef->nparallels;
@@ -10325,6 +10330,29 @@ virDomainChrGetDomainPtrs(virDomainDefPtr vmdef,
}
}
+/* Return the array within vmdef that can contain a chrdefptr of the
+ * given type. */
+void
+virDomainChrGetDomainPtrs(const virDomainDef *vmdef,
+ enum virDomainChrDeviceType type,
+ const virDomainChrDef ***arrPtr,
+ size_t *cntPtr)
+{
+ virDomainChrDef ***arrVar;
+ size_t *cntVar;
+
+ /* Cast away const; we add it back in the final assignment. */
+ virDomainChrGetDomainPtrsInternal((virDomainDefPtr) vmdef, type,
+ &arrVar, &cntVar);
+ if (arrVar) {
+ *arrPtr = (const virDomainChrDef **) *arrVar;
+ *cntPtr = *cntVar;
+ } else {
+ *arrPtr = NULL;
+ *cntPtr = 0;
+ }
+}
+
int
virDomainChrInsert(virDomainDefPtr vmdef,
virDomainChrDefPtr chr)
@@ -10332,7 +10360,7 @@ virDomainChrInsert(virDomainDefPtr vmdef,
virDomainChrDefPtr **arrPtr;
size_t *cntPtr;
- virDomainChrGetDomainPtrs(vmdef, chr, &arrPtr, &cntPtr);
+ virDomainChrGetDomainPtrsInternal(vmdef, chr->deviceType, &arrPtr,
&cntPtr);
return VIR_APPEND_ELEMENT(*arrPtr, *cntPtr, chr);
}
@@ -10344,7 +10372,7 @@ virDomainChrRemove(virDomainDefPtr vmdef,
virDomainChrDefPtr ret, **arrPtr;
size_t i, *cntPtr;
- virDomainChrGetDomainPtrs(vmdef, chr, &arrPtr, &cntPtr);
+ virDomainChrGetDomainPtrsInternal(vmdef, chr->deviceType, &arrPtr,
&cntPtr);
for (i = 0; i < *cntPtr; i++) {
ret = (*arrPtr)[i];
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f20a916..779a429 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2439,10 +2439,11 @@ virDomainLeaseRemove(virDomainDefPtr def,
virDomainLeaseDefPtr lease);
void
-virDomainChrGetDomainPtrs(virDomainDefPtr vmdef,
- virDomainChrDefPtr chr,
- virDomainChrDefPtr ***arrPtr,
- size_t **cntPtr);
+virDomainChrGetDomainPtrs(const virDomainDef *vmdef,
+ enum virDomainChrDeviceType type,
+ const virDomainChrDef ***arrPtr,
+ size_t *cntPtr)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
virDomainChrDefPtr
virDomainChrFind(virDomainDefPtr def,
virDomainChrDefPtr target);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 52dc295..adb065e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -584,7 +584,7 @@ cleanup:
return ret;
}
-static int qemuDomainDeviceAliasIndex(virDomainDeviceInfoPtr info,
+static int qemuDomainDeviceAliasIndex(const virDomainDeviceInfo *info,
const char *prefix)
{
int idx;
@@ -911,8 +911,8 @@ qemuGetNextChrDevIndex(virDomainDefPtr def,
virDomainChrDefPtr chr,
const char *prefix)
{
- virDomainChrDefPtr **arrPtr;
- size_t *cntPtr;
+ const virDomainChrDef **arrPtr;
+ size_t cnt;
size_t i;
ssize_t idx = 0;
const char *prefix2 = NULL;
@@ -920,13 +920,13 @@ qemuGetNextChrDevIndex(virDomainDefPtr def,
if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE)
prefix2 = "serial";
- virDomainChrGetDomainPtrs(def, chr, &arrPtr, &cntPtr);
+ virDomainChrGetDomainPtrs(def, chr->deviceType, &arrPtr, &cnt);
- for (i = 0; i < *cntPtr; i++) {
+ for (i = 0; i < cnt; i++) {
ssize_t thisidx;
- if (((thisidx = qemuDomainDeviceAliasIndex(&(*arrPtr)[i]->info, prefix))
< 0) &&
+ if (((thisidx = qemuDomainDeviceAliasIndex(&arrPtr[i]->info, prefix)) <
0) &&
(prefix2 &&
- (thisidx = qemuDomainDeviceAliasIndex(&(*arrPtr)[i]->info, prefix2))
< 0)) {
+ (thisidx = qemuDomainDeviceAliasIndex(&arrPtr[i]->info, prefix2))
< 0)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Unable to determine device index for character
device"));
return -1;
--
1.8.3.1