[libvirt PATCH 0/2] Random improvements to work around a build system issue

This is an alternative to https://listman.redhat.com/archives/libvir-list/2021-September/msg00522.html. When libvirt is build: * with sanitizers enabled, * buildtype explicitly set to "debug", * on clang, the build fails with: ../src/conf/nwfilter_conf.c:2190:1: error: stack frame size of 10616 bytes in function 'virNWFilterRuleDefFixup' [-Werror,-Wframe-larger-than=] virNWFilterRuleDefFixup(virNWFilterRuleDef *rule) ^ 1 error generated. ../src/conf/domain_conf.c:19514:1: error: stack frame size of 8312 bytes in function 'virDomainDefParseXML' [-Werror,-Wframe-larger-than=] virDomainDefParseXML(xmlXPathContextPtr ctxt, ^ 1 error generated. Note that this does not happen when "-Dbuildtype" is not specified, even though "debug" is the default build type. The patches in this series happen to make these errors go away. Regards, Tim Tim Wiederhake (2): virDomainDefParseXML: Use automatic memory management virNWFilterRuleDefFixup: Replace macro with function src/conf/domain_conf.c | 208 ++++++++++++++-------------- src/conf/nwfilter_conf.c | 284 ++++++++++++++++++++------------------- 2 files changed, 245 insertions(+), 247 deletions(-) -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 208 ++++++++++++++++++++--------------------- 1 file changed, 102 insertions(+), 106 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6c32609431..cdbc66d9fc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19518,27 +19518,27 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, xmlNodePtr node = NULL; size_t i, j; int n; - virDomainDef *def; bool uuid_generated = false; bool usb_none = false; g_autofree xmlNodePtr *nodes = NULL; g_autofree char *tmp = NULL; + g_autoptr(virDomainDef) def = NULL; if (!(def = virDomainDefNew(xmlopt))) return NULL; if (virDomainDefParseIDs(def, ctxt, flags, &uuid_generated) < 0) - goto error; + return NULL; if (virDomainDefParseCaps(def, ctxt, xmlopt) < 0) - goto error; + return NULL; /* Extract short description of domain (title) */ def->title = virXPathString("string(./title[1])", ctxt); if (def->title && strchr(def->title, '\n')) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Domain title can't contain newlines")); - goto error; + return NULL; } /* Extract documentation if present */ @@ -19548,40 +19548,40 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, * late, so devices can refer to this for defaults */ if (!(flags & VIR_DOMAIN_DEF_PARSE_SKIP_SECLABEL)) { if (virSecurityLabelDefsParseXML(def, ctxt, xmlopt, flags) == -1) - goto error; + return NULL; } if (virDomainDefParseMemory(def, ctxt) < 0) - goto error; + return NULL; if (virDomainDefTunablesParse(def, ctxt, xmlopt, flags) < 0) - goto error; + return NULL; if (virCPUDefParseXML(ctxt, "./cpu[1]", VIR_CPU_TYPE_GUEST, &def->cpu, false) < 0) - goto error; + return NULL; if (virDomainNumaDefParseXML(def->numa, ctxt) < 0) - goto error; + return NULL; if (virDomainNumaGetCPUCountTotal(def->numa) > virDomainDefGetVcpusMax(def)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Number of CPUs in <numa> exceeds the" " <vcpu> count")); - goto error; + return NULL; } if (virDomainNumaGetMaxCPUID(def->numa) >= virDomainDefGetVcpusMax(def)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("CPU IDs in <numa> exceed the <vcpu> count")); - goto error; + return NULL; } if (virDomainNumatuneParseXML(def->numa, def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC, ctxt) < 0) - goto error; + return NULL; if (virDomainNumatuneHasPlacementAuto(def->numa) && !def->cpumask && !virDomainDefHasVcpuPin(def) && @@ -19592,38 +19592,38 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, if ((n = virXPathNodeSet("./resource", ctxt, &nodes)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot extract resource nodes")); - goto error; + return NULL; } if (n > 1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("only one resource element is supported")); - goto error; + return NULL; } if (n && !(def->resource = virDomainResourceDefParse(nodes[0], ctxt))) - goto error; + return NULL; VIR_FREE(nodes); if (virDomainFeaturesDefParse(def, ctxt) < 0) - goto error; + return NULL; if (virDomainDefLifecycleParse(def, ctxt) < 0) - goto error; + return NULL; if (virDomainPerfDefParseXML(def, ctxt) < 0) - goto error; + return NULL; if (virDomainDefClockParse(def, ctxt) < 0) - goto error; + return NULL; if (virDomainDefParseBootOptions(def, ctxt) < 0) - goto error; + return NULL; /* analysis of the disk devices */ if ((n = virXPathNodeSet("./devices/disk", ctxt, &nodes)) < 0) - goto error; + return NULL; for (i = 0; i < n; i++) { virDomainDiskDef *disk = virDomainDiskDefParseXML(xmlopt, @@ -19631,27 +19631,27 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, ctxt, flags); if (!disk) - goto error; + return NULL; virDomainDiskInsert(def, disk); } VIR_FREE(nodes); if (virDomainDefControllersParse(def, ctxt, xmlopt, flags, &usb_none) < 0) - goto error; + return NULL; /* analysis of the resource leases */ if ((n = virXPathNodeSet("./devices/lease", ctxt, &nodes)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot extract device leases")); - goto error; + return NULL; } if (n) def->leases = g_new0(virDomainLeaseDef *, n); for (i = 0; i < n; i++) { virDomainLeaseDef *lease = virDomainLeaseDefParseXML(nodes[i], ctxt); if (!lease) - goto error; + return NULL; def->leases[def->nleases++] = lease; } @@ -19659,7 +19659,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* analysis of the filesystems */ if ((n = virXPathNodeSet("./devices/filesystem", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->fss = g_new0(virDomainFSDef *, n); for (i = 0; i < n; i++) { @@ -19668,7 +19668,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, ctxt, flags); if (!fs) - goto error; + return NULL; def->fss[def->nfss++] = fs; } @@ -19676,7 +19676,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* analysis of the network devices */ if ((n = virXPathNodeSet("./devices/interface", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->nets = g_new0(virDomainNetDef *, n); for (i = 0; i < n; i++) { @@ -19685,7 +19685,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, ctxt, flags); if (!net) - goto error; + return NULL; def->nets[def->nnets++] = net; @@ -19695,7 +19695,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, */ if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV && virDomainHostdevInsert(def, virDomainNetGetActualHostdev(net)) < 0) { - goto error; + return NULL; } } VIR_FREE(nodes); @@ -19703,7 +19703,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* analysis of the smartcard devices */ if ((n = virXPathNodeSet("./devices/smartcard", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->smartcards = g_new0(virDomainSmartcardDef *, n); @@ -19713,7 +19713,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, ctxt, flags); if (!card) - goto error; + return NULL; def->smartcards[def->nsmartcards++] = card; } @@ -19722,7 +19722,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* analysis of the character devices */ if ((n = virXPathNodeSet("./devices/parallel", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->parallels = g_new0(virDomainChrDef *, n); @@ -19732,7 +19732,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, nodes[i], flags); if (!chr) - goto error; + return NULL; if (chr->target.port == -1) { int maxport = -1; @@ -19747,7 +19747,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, VIR_FREE(nodes); if ((n = virXPathNodeSet("./devices/serial", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->serials = g_new0(virDomainChrDef *, n); @@ -19758,7 +19758,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, nodes[i], flags); if (!chr) - goto error; + return NULL; if (chr->target.port == -1) { int maxport = -1; @@ -19775,7 +19775,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, if ((n = virXPathNodeSet("./devices/console", ctxt, &nodes)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot extract console devices")); - goto error; + return NULL; } if (n) def->consoles = g_new0(virDomainChrDef *, n); @@ -19786,7 +19786,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, nodes[i], flags); if (!chr) - goto error; + return NULL; chr->target.port = i; def->consoles[def->nconsoles++] = chr; @@ -19794,7 +19794,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, VIR_FREE(nodes); if ((n = virXPathNodeSet("./devices/channel", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->channels = g_new0(virDomainChrDef *, n); @@ -19804,7 +19804,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, nodes[i], flags); if (!chr) - goto error; + return NULL; def->channels[def->nchannels++] = chr; } @@ -19813,7 +19813,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* analysis of the input devices */ if ((n = virXPathNodeSet("./devices/input", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->inputs = g_new0(virDomainInputDef *, n); @@ -19824,7 +19824,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, ctxt, flags); if (!input) - goto error; + return NULL; /* Check if USB bus is required */ if (input->bus == VIR_DOMAIN_INPUT_BUS_USB && usb_none) { @@ -19832,7 +19832,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Can't add USB input device. " "USB bus is disabled")); - goto error; + return NULL; } def->inputs[def->ninputs++] = input; @@ -19841,7 +19841,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* analysis of the graphics devices */ if ((n = virXPathNodeSet("./devices/graphics", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->graphics = g_new0(virDomainGraphicsDef *, n); for (i = 0; i < n; i++) { @@ -19850,7 +19850,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, ctxt, flags); if (!graphics) - goto error; + return NULL; def->graphics[def->ngraphics++] = graphics; } @@ -19858,7 +19858,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* analysis of the sound devices */ if ((n = virXPathNodeSet("./devices/sound", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->sounds = g_new0(virDomainSoundDef *, n); for (i = 0; i < n; i++) { @@ -19867,7 +19867,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, ctxt, flags); if (!sound) - goto error; + return NULL; def->sounds[def->nsounds++] = sound; } @@ -19875,7 +19875,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* analysis of the audio devices */ if ((n = virXPathNodeSet("./devices/audio", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->audios = g_new0(virDomainAudioDef *, n); for (i = 0; i < n; i++) { @@ -19883,7 +19883,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, nodes[i], ctxt); if (!audio) - goto error; + return NULL; def->audios[def->naudios++] = audio; } @@ -19891,7 +19891,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* analysis of the video devices */ if ((n = virXPathNodeSet("./devices/video", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->videos = g_new0(virDomainVideoDef *, n); for (i = 0; i < n; i++) { @@ -19900,7 +19900,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, if (!(video = virDomainVideoDefParseXML(xmlopt, nodes[i], ctxt, flags))) - goto error; + return NULL; if (video->primary) { insertAt = 0; @@ -19910,7 +19910,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, insertAt, def->nvideos, video) < 0) { - goto error; + return NULL; } } @@ -19918,7 +19918,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* analysis of the host devices */ if ((n = virXPathNodeSet("./devices/hostdev", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n > 0) VIR_REALLOC_N(def->hostdevs, def->nhostdevs + n); @@ -19928,7 +19928,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, hostdev = virDomainHostdevDefParseXML(xmlopt, nodes[i], ctxt, flags); if (!hostdev) - goto error; + return NULL; if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB && usb_none) { @@ -19936,7 +19936,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, _("Can't add host USB device: " "USB is disabled in this host")); virDomainHostdevDefFree(hostdev); - goto error; + return NULL; } def->hostdevs[def->nhostdevs++] = hostdev; @@ -19948,25 +19948,25 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, * load the controller during hostdev hotplug. */ if (virDomainDefMaybeAddHostdevSCSIcontroller(def) < 0) - goto error; + return NULL; } VIR_FREE(nodes); /* analysis of the watchdog devices */ def->watchdog = NULL; if ((n = virXPathNodeSet("./devices/watchdog", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n > 1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("only a single watchdog device is supported")); - goto error; + return NULL; } if (n > 0) { virDomainWatchdogDef *watchdog; watchdog = virDomainWatchdogDefParseXML(xmlopt, nodes[0], ctxt, flags); if (!watchdog) - goto error; + return NULL; def->watchdog = watchdog; VIR_FREE(nodes); @@ -19975,18 +19975,18 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* analysis of the memballoon devices */ def->memballoon = NULL; if ((n = virXPathNodeSet("./devices/memballoon", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n > 1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("only a single memory balloon device is supported")); - goto error; + return NULL; } if (n > 0) { virDomainMemballoonDef *memballoon; memballoon = virDomainMemballoonDefParseXML(xmlopt, nodes[0], ctxt, flags); if (!memballoon) - goto error; + return NULL; def->memballoon = memballoon; VIR_FREE(nodes); @@ -19994,14 +19994,14 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* Parse the RNG devices */ if ((n = virXPathNodeSet("./devices/rng", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->rngs = g_new0(virDomainRNGDef *, n); for (i = 0; i < n; i++) { virDomainRNGDef *rng = virDomainRNGDefParseXML(xmlopt, nodes[i], ctxt, flags); if (!rng) - goto error; + return NULL; def->rngs[def->nrngs++] = rng; } @@ -20009,13 +20009,13 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* Parse the TPM devices */ if ((n = virXPathNodeSet("./devices/tpm", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n > 2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("a maximum of two TPM devices is supported, one of " "them being a TPM Proxy device")); - goto error; + return NULL; } if (n) @@ -20025,31 +20025,31 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, virDomainTPMDef *tpm = virDomainTPMDefParseXML(xmlopt, nodes[i], ctxt, flags); if (!tpm) - goto error; + return NULL; def->tpms[def->ntpms++] = tpm; } VIR_FREE(nodes); if ((n = virXPathNodeSet("./devices/nvram", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n > 1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("only a single nvram device is supported")); - goto error; + return NULL; } else if (n == 1) { virDomainNVRAMDef *nvram = virDomainNVRAMDefParseXML(xmlopt, nodes[0], ctxt, flags); if (!nvram) - goto error; + return NULL; def->nvram = nvram; VIR_FREE(nodes); } /* analysis of the hub devices */ if ((n = virXPathNodeSet("./devices/hub", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->hubs = g_new0(virDomainHubDef *, n); for (i = 0; i < n; i++) { @@ -20057,14 +20057,14 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, hub = virDomainHubDefParseXML(xmlopt, nodes[i], ctxt, flags); if (!hub) - goto error; + return NULL; if (hub->type == VIR_DOMAIN_HUB_TYPE_USB && usb_none) { virDomainHubDefFree(hub); virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Can't add USB hub: " "USB is disabled for this domain")); - goto error; + return NULL; } def->hubs[def->nhubs++] = hub; @@ -20073,14 +20073,14 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* analysis of the redirected devices */ if ((n = virXPathNodeSet("./devices/redirdev", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->redirdevs = g_new0(virDomainRedirdevDef *, n); for (i = 0; i < n; i++) { virDomainRedirdevDef *redirdev = virDomainRedirdevDefParseXML(xmlopt, nodes[i], ctxt, flags); if (!redirdev) - goto error; + return NULL; def->redirdevs[def->nredirdevs++] = redirdev; } @@ -20088,18 +20088,18 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* analysis of the redirection filter rules */ if ((n = virXPathNodeSet("./devices/redirfilter", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n > 1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("only one set of redirection filter rule is supported")); - goto error; + return NULL; } if (n) { virDomainRedirFilterDef *redirfilter = virDomainRedirFilterDefParseXML(nodes[0], ctxt); if (!redirfilter) - goto error; + return NULL; def->redirfilter = redirfilter; } @@ -20107,7 +20107,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* analysis of the panic devices */ if ((n = virXPathNodeSet("./devices/panic", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->panics = g_new0(virDomainPanicDef *, n); for (i = 0; i < n; i++) { @@ -20115,7 +20115,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, panic = virDomainPanicDefParseXML(xmlopt, nodes[i], ctxt, flags); if (!panic) - goto error; + return NULL; def->panics[def->npanics++] = panic; } @@ -20123,7 +20123,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, /* analysis of the shmem devices */ if ((n = virXPathNodeSet("./devices/shmem", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->shmems = g_new0(virDomainShmemDef *, n); @@ -20133,7 +20133,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, ctxt->node = nodes[i]; shmem = virDomainShmemDefParseXML(xmlopt, nodes[i], ctxt, flags); if (!shmem) - goto error; + return NULL; def->shmems[def->nshmems++] = shmem; } @@ -20144,12 +20144,12 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, if ((node = virXPathNode("./launchSecurity", ctxt)) != NULL) { def->sec = virDomainSecDefParseXML(node, ctxt); if (!def->sec) - goto error; + return NULL; } /* analysis of memory devices */ if ((n = virXPathNodeSet("./devices/memory", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) def->mems = g_new0(virDomainMemoryDef *, n); @@ -20159,70 +20159,70 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, ctxt, flags); if (!mem) - goto error; + return NULL; def->mems[def->nmems++] = mem; } VIR_FREE(nodes); if ((n = virXPathNodeSet("./devices/iommu", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n > 1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("only a single IOMMU device is supported")); - goto error; + return NULL; } if (n > 0) { if (!(def->iommu = virDomainIOMMUDefParseXML(nodes[0], ctxt))) - goto error; + return NULL; } VIR_FREE(nodes); if ((n = virXPathNodeSet("./devices/vsock", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n > 1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("only a single vsock device is supported")); - goto error; + return NULL; } if (n > 0) { if (!(def->vsock = virDomainVsockDefParseXML(xmlopt, nodes[0], ctxt, flags))) - goto error; + return NULL; } VIR_FREE(nodes); /* analysis of the user namespace mapping */ if ((n = virXPathNodeSet("./idmap/uid", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) { def->idmap.uidmap = virDomainIdmapDefParseXML(ctxt, nodes, n); if (!def->idmap.uidmap) - goto error; + return NULL; def->idmap.nuidmap = n; } VIR_FREE(nodes); if ((n = virXPathNodeSet("./idmap/gid", ctxt, &nodes)) < 0) - goto error; + return NULL; if (n) { def->idmap.gidmap = virDomainIdmapDefParseXML(ctxt, nodes, n); if (!def->idmap.gidmap) - goto error; + return NULL; def->idmap.ngidmap = n; } VIR_FREE(nodes); if ((n = virXPathNodeSet("./sysinfo", ctxt, &nodes)) < 0) - goto error; + return NULL; def->sysinfo = g_new0(virSysinfoDef *, n); @@ -20231,7 +20231,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, def->uuid, uuid_generated); if (!sysinfo) - goto error; + return NULL; def->sysinfo[def->nsysinfo++] = sysinfo; } @@ -20243,13 +20243,13 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, if ((mode = virDomainSmbiosModeTypeFromString(tmp)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown smbios mode '%s'"), tmp); - goto error; + return NULL; } def->os.smbios_mode = mode; } if (virDomainKeyWrapDefParseXML(def, ctxt) < 0) - goto error; + return NULL; /* Extract custom metadata */ if ((node = virXPathNode("./metadata[1]", ctxt)) != NULL) @@ -20262,16 +20262,12 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, if (def->ns.parse) { if (virXMLNamespaceRegister(ctxt, &def->ns) < 0) - goto error; + return NULL; if ((def->ns.parse)(ctxt, &def->namespaceData) < 0) - goto error; + return NULL; } - return def; - - error: - virDomainDefFree(def); - return NULL; + return g_steal_pointer(&def); } -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/nwfilter_conf.c | 284 ++++++++++++++++++++------------------- 1 file changed, 143 insertions(+), 141 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index a3109962af..4c4e31d5cd 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2187,71 +2187,74 @@ virNWFilterRuleValidate(virNWFilterRuleDef *rule) static void -virNWFilterRuleDefFixup(virNWFilterRuleDef *rule) +copy_neg_sign(nwItemDesc *to, const nwItemDesc *from) { -#define COPY_NEG_SIGN(A, B) \ - (A).flags = ((A).flags & ~NWFILTER_ENTRY_ITEM_FLAG_IS_NEG) | \ - ((B).flags & NWFILTER_ENTRY_ITEM_FLAG_IS_NEG); + to->flags = (to->flags & ~NWFILTER_ENTRY_ITEM_FLAG_IS_NEG) | + (from->flags & NWFILTER_ENTRY_ITEM_FLAG_IS_NEG); +} +static void +virNWFilterRuleDefFixup(virNWFilterRuleDef *rule) +{ switch (rule->prtclType) { case VIR_NWFILTER_RULE_PROTOCOL_MAC: - COPY_NEG_SIGN(rule->p.ethHdrFilter.ethHdr.dataSrcMACMask, - rule->p.ethHdrFilter.ethHdr.dataSrcMACAddr); - COPY_NEG_SIGN(rule->p.ethHdrFilter.ethHdr.dataDstMACMask, - rule->p.ethHdrFilter.ethHdr.dataDstMACAddr); + copy_neg_sign(&rule->p.ethHdrFilter.ethHdr.dataSrcMACMask, + &rule->p.ethHdrFilter.ethHdr.dataSrcMACAddr); + copy_neg_sign(&rule->p.ethHdrFilter.ethHdr.dataDstMACMask, + &rule->p.ethHdrFilter.ethHdr.dataDstMACAddr); break; case VIR_NWFILTER_RULE_PROTOCOL_VLAN: - COPY_NEG_SIGN(rule->p.vlanHdrFilter.ethHdr.dataSrcMACMask, - rule->p.vlanHdrFilter.ethHdr.dataSrcMACAddr); - COPY_NEG_SIGN(rule->p.vlanHdrFilter.ethHdr.dataDstMACMask, - rule->p.vlanHdrFilter.ethHdr.dataDstMACAddr); + copy_neg_sign(&rule->p.vlanHdrFilter.ethHdr.dataSrcMACMask, + &rule->p.vlanHdrFilter.ethHdr.dataSrcMACAddr); + copy_neg_sign(&rule->p.vlanHdrFilter.ethHdr.dataDstMACMask, + &rule->p.vlanHdrFilter.ethHdr.dataDstMACAddr); break; case VIR_NWFILTER_RULE_PROTOCOL_STP: - COPY_NEG_SIGN(rule->p.stpHdrFilter.ethHdr.dataSrcMACMask, - rule->p.stpHdrFilter.ethHdr.dataSrcMACAddr); - COPY_NEG_SIGN(rule->p.stpHdrFilter.dataRootPriHi, - rule->p.stpHdrFilter.dataRootPri); - COPY_NEG_SIGN(rule->p.stpHdrFilter.dataRootAddrMask, - rule->p.stpHdrFilter.dataRootAddr); - COPY_NEG_SIGN(rule->p.stpHdrFilter.dataRootCostHi, - rule->p.stpHdrFilter.dataRootCost); - COPY_NEG_SIGN(rule->p.stpHdrFilter.dataSndrPrioHi, - rule->p.stpHdrFilter.dataSndrPrio); - COPY_NEG_SIGN(rule->p.stpHdrFilter.dataSndrAddrMask, - rule->p.stpHdrFilter.dataSndrAddr); - COPY_NEG_SIGN(rule->p.stpHdrFilter.dataPortHi, - rule->p.stpHdrFilter.dataPort); - COPY_NEG_SIGN(rule->p.stpHdrFilter.dataAgeHi, - rule->p.stpHdrFilter.dataAge); - COPY_NEG_SIGN(rule->p.stpHdrFilter.dataMaxAgeHi, - rule->p.stpHdrFilter.dataMaxAge); - COPY_NEG_SIGN(rule->p.stpHdrFilter.dataHelloTimeHi, - rule->p.stpHdrFilter.dataHelloTime); - COPY_NEG_SIGN(rule->p.stpHdrFilter.dataFwdDelayHi, - rule->p.stpHdrFilter.dataFwdDelay); + copy_neg_sign(&rule->p.stpHdrFilter.ethHdr.dataSrcMACMask, + &rule->p.stpHdrFilter.ethHdr.dataSrcMACAddr); + copy_neg_sign(&rule->p.stpHdrFilter.dataRootPriHi, + &rule->p.stpHdrFilter.dataRootPri); + copy_neg_sign(&rule->p.stpHdrFilter.dataRootAddrMask, + &rule->p.stpHdrFilter.dataRootAddr); + copy_neg_sign(&rule->p.stpHdrFilter.dataRootCostHi, + &rule->p.stpHdrFilter.dataRootCost); + copy_neg_sign(&rule->p.stpHdrFilter.dataSndrPrioHi, + &rule->p.stpHdrFilter.dataSndrPrio); + copy_neg_sign(&rule->p.stpHdrFilter.dataSndrAddrMask, + &rule->p.stpHdrFilter.dataSndrAddr); + copy_neg_sign(&rule->p.stpHdrFilter.dataPortHi, + &rule->p.stpHdrFilter.dataPort); + copy_neg_sign(&rule->p.stpHdrFilter.dataAgeHi, + &rule->p.stpHdrFilter.dataAge); + copy_neg_sign(&rule->p.stpHdrFilter.dataMaxAgeHi, + &rule->p.stpHdrFilter.dataMaxAge); + copy_neg_sign(&rule->p.stpHdrFilter.dataHelloTimeHi, + &rule->p.stpHdrFilter.dataHelloTime); + copy_neg_sign(&rule->p.stpHdrFilter.dataFwdDelayHi, + &rule->p.stpHdrFilter.dataFwdDelay); break; case VIR_NWFILTER_RULE_PROTOCOL_IP: - COPY_NEG_SIGN(rule->p.ipHdrFilter.ipHdr.dataSrcIPMask, - rule->p.ipHdrFilter.ipHdr.dataSrcIPAddr); - COPY_NEG_SIGN(rule->p.ipHdrFilter.ipHdr.dataDstIPMask, - rule->p.ipHdrFilter.ipHdr.dataDstIPAddr); + copy_neg_sign(&rule->p.ipHdrFilter.ipHdr.dataSrcIPMask, + &rule->p.ipHdrFilter.ipHdr.dataSrcIPAddr); + copy_neg_sign(&rule->p.ipHdrFilter.ipHdr.dataDstIPMask, + &rule->p.ipHdrFilter.ipHdr.dataDstIPAddr); virNWFilterRuleDefFixupIPSet(&rule->p.ipHdrFilter.ipHdr); break; case VIR_NWFILTER_RULE_PROTOCOL_IPV6: - COPY_NEG_SIGN(rule->p.ipv6HdrFilter.ipHdr.dataSrcIPMask, - rule->p.ipv6HdrFilter.ipHdr.dataSrcIPAddr); - COPY_NEG_SIGN(rule->p.ipv6HdrFilter.ipHdr.dataDstIPMask, - rule->p.ipv6HdrFilter.ipHdr.dataDstIPAddr); - COPY_NEG_SIGN(rule->p.ipv6HdrFilter.dataICMPTypeEnd, - rule->p.ipv6HdrFilter.dataICMPTypeStart); - COPY_NEG_SIGN(rule->p.ipv6HdrFilter.dataICMPCodeStart, - rule->p.ipv6HdrFilter.dataICMPTypeStart); - COPY_NEG_SIGN(rule->p.ipv6HdrFilter.dataICMPCodeEnd, - rule->p.ipv6HdrFilter.dataICMPTypeStart); + copy_neg_sign(&rule->p.ipv6HdrFilter.ipHdr.dataSrcIPMask, + &rule->p.ipv6HdrFilter.ipHdr.dataSrcIPAddr); + copy_neg_sign(&rule->p.ipv6HdrFilter.ipHdr.dataDstIPMask, + &rule->p.ipv6HdrFilter.ipHdr.dataDstIPAddr); + copy_neg_sign(&rule->p.ipv6HdrFilter.dataICMPTypeEnd, + &rule->p.ipv6HdrFilter.dataICMPTypeStart); + copy_neg_sign(&rule->p.ipv6HdrFilter.dataICMPCodeStart, + &rule->p.ipv6HdrFilter.dataICMPTypeStart); + copy_neg_sign(&rule->p.ipv6HdrFilter.dataICMPCodeEnd, + &rule->p.ipv6HdrFilter.dataICMPTypeStart); virNWFilterRuleDefFixupIPSet(&rule->p.ipv6HdrFilter.ipHdr); break; @@ -2262,143 +2265,142 @@ virNWFilterRuleDefFixup(virNWFilterRuleDef *rule) case VIR_NWFILTER_RULE_PROTOCOL_TCP: case VIR_NWFILTER_RULE_PROTOCOL_TCPoIPV6: - COPY_NEG_SIGN(rule->p.tcpHdrFilter.ipHdr.dataSrcIPMask, - rule->p.tcpHdrFilter.ipHdr.dataSrcIPAddr); - COPY_NEG_SIGN(rule->p.tcpHdrFilter.ipHdr.dataDstIPMask, - rule->p.tcpHdrFilter.ipHdr.dataDstIPAddr); - COPY_NEG_SIGN(rule->p.tcpHdrFilter.ipHdr.dataSrcIPTo, - rule->p.tcpHdrFilter.ipHdr.dataSrcIPFrom); - COPY_NEG_SIGN(rule->p.tcpHdrFilter.ipHdr.dataDstIPTo, - rule->p.tcpHdrFilter.ipHdr.dataDstIPFrom); - COPY_NEG_SIGN(rule->p.tcpHdrFilter.portData.dataSrcPortEnd, - rule->p.tcpHdrFilter.portData.dataSrcPortStart); - COPY_NEG_SIGN(rule->p.tcpHdrFilter.portData.dataDstPortStart, - rule->p.tcpHdrFilter.portData.dataSrcPortStart); - COPY_NEG_SIGN(rule->p.tcpHdrFilter.portData.dataDstPortEnd, - rule->p.tcpHdrFilter.portData.dataSrcPortStart); + copy_neg_sign(&rule->p.tcpHdrFilter.ipHdr.dataSrcIPMask, + &rule->p.tcpHdrFilter.ipHdr.dataSrcIPAddr); + copy_neg_sign(&rule->p.tcpHdrFilter.ipHdr.dataDstIPMask, + &rule->p.tcpHdrFilter.ipHdr.dataDstIPAddr); + copy_neg_sign(&rule->p.tcpHdrFilter.ipHdr.dataSrcIPTo, + &rule->p.tcpHdrFilter.ipHdr.dataSrcIPFrom); + copy_neg_sign(&rule->p.tcpHdrFilter.ipHdr.dataDstIPTo, + &rule->p.tcpHdrFilter.ipHdr.dataDstIPFrom); + copy_neg_sign(&rule->p.tcpHdrFilter.portData.dataSrcPortEnd, + &rule->p.tcpHdrFilter.portData.dataSrcPortStart); + copy_neg_sign(&rule->p.tcpHdrFilter.portData.dataDstPortStart, + &rule->p.tcpHdrFilter.portData.dataSrcPortStart); + copy_neg_sign(&rule->p.tcpHdrFilter.portData.dataDstPortEnd, + &rule->p.tcpHdrFilter.portData.dataSrcPortStart); virNWFilterRuleDefFixupIPSet(&rule->p.tcpHdrFilter.ipHdr); break; case VIR_NWFILTER_RULE_PROTOCOL_UDP: case VIR_NWFILTER_RULE_PROTOCOL_UDPoIPV6: - COPY_NEG_SIGN(rule->p.udpHdrFilter.ipHdr.dataSrcIPMask, - rule->p.udpHdrFilter.ipHdr.dataSrcIPAddr); - COPY_NEG_SIGN(rule->p.udpHdrFilter.ipHdr.dataDstIPMask, - rule->p.udpHdrFilter.ipHdr.dataDstIPAddr); - COPY_NEG_SIGN(rule->p.udpHdrFilter.ipHdr.dataSrcIPTo, - rule->p.udpHdrFilter.ipHdr.dataSrcIPFrom); - COPY_NEG_SIGN(rule->p.udpHdrFilter.ipHdr.dataDstIPTo, - rule->p.udpHdrFilter.ipHdr.dataDstIPFrom); - COPY_NEG_SIGN(rule->p.udpHdrFilter.portData.dataSrcPortEnd, - rule->p.udpHdrFilter.portData.dataSrcPortStart); - COPY_NEG_SIGN(rule->p.udpHdrFilter.portData.dataDstPortStart, - rule->p.udpHdrFilter.portData.dataSrcPortStart); - COPY_NEG_SIGN(rule->p.udpHdrFilter.portData.dataDstPortEnd, - rule->p.udpHdrFilter.portData.dataSrcPortStart); + copy_neg_sign(&rule->p.udpHdrFilter.ipHdr.dataSrcIPMask, + &rule->p.udpHdrFilter.ipHdr.dataSrcIPAddr); + copy_neg_sign(&rule->p.udpHdrFilter.ipHdr.dataDstIPMask, + &rule->p.udpHdrFilter.ipHdr.dataDstIPAddr); + copy_neg_sign(&rule->p.udpHdrFilter.ipHdr.dataSrcIPTo, + &rule->p.udpHdrFilter.ipHdr.dataSrcIPFrom); + copy_neg_sign(&rule->p.udpHdrFilter.ipHdr.dataDstIPTo, + &rule->p.udpHdrFilter.ipHdr.dataDstIPFrom); + copy_neg_sign(&rule->p.udpHdrFilter.portData.dataSrcPortEnd, + &rule->p.udpHdrFilter.portData.dataSrcPortStart); + copy_neg_sign(&rule->p.udpHdrFilter.portData.dataDstPortStart, + &rule->p.udpHdrFilter.portData.dataSrcPortStart); + copy_neg_sign(&rule->p.udpHdrFilter.portData.dataDstPortEnd, + &rule->p.udpHdrFilter.portData.dataSrcPortStart); virNWFilterRuleDefFixupIPSet(&rule->p.udpHdrFilter.ipHdr); break; case VIR_NWFILTER_RULE_PROTOCOL_UDPLITE: case VIR_NWFILTER_RULE_PROTOCOL_UDPLITEoIPV6: - COPY_NEG_SIGN(rule->p.udpliteHdrFilter.ipHdr.dataSrcIPMask, - rule->p.udpliteHdrFilter.ipHdr.dataSrcIPAddr); - COPY_NEG_SIGN(rule->p.udpliteHdrFilter.ipHdr.dataDstIPMask, - rule->p.udpliteHdrFilter.ipHdr.dataDstIPAddr); - COPY_NEG_SIGN(rule->p.udpliteHdrFilter.ipHdr.dataSrcIPTo, - rule->p.udpliteHdrFilter.ipHdr.dataSrcIPFrom); - COPY_NEG_SIGN(rule->p.udpliteHdrFilter.ipHdr.dataDstIPTo, - rule->p.udpliteHdrFilter.ipHdr.dataDstIPFrom); + copy_neg_sign(&rule->p.udpliteHdrFilter.ipHdr.dataSrcIPMask, + &rule->p.udpliteHdrFilter.ipHdr.dataSrcIPAddr); + copy_neg_sign(&rule->p.udpliteHdrFilter.ipHdr.dataDstIPMask, + &rule->p.udpliteHdrFilter.ipHdr.dataDstIPAddr); + copy_neg_sign(&rule->p.udpliteHdrFilter.ipHdr.dataSrcIPTo, + &rule->p.udpliteHdrFilter.ipHdr.dataSrcIPFrom); + copy_neg_sign(&rule->p.udpliteHdrFilter.ipHdr.dataDstIPTo, + &rule->p.udpliteHdrFilter.ipHdr.dataDstIPFrom); virNWFilterRuleDefFixupIPSet(&rule->p.udpliteHdrFilter.ipHdr); break; case VIR_NWFILTER_RULE_PROTOCOL_ESP: case VIR_NWFILTER_RULE_PROTOCOL_ESPoIPV6: - COPY_NEG_SIGN(rule->p.espHdrFilter.ipHdr.dataSrcIPMask, - rule->p.espHdrFilter.ipHdr.dataSrcIPAddr); - COPY_NEG_SIGN(rule->p.espHdrFilter.ipHdr.dataDstIPMask, - rule->p.espHdrFilter.ipHdr.dataDstIPAddr); - COPY_NEG_SIGN(rule->p.espHdrFilter.ipHdr.dataSrcIPTo, - rule->p.espHdrFilter.ipHdr.dataSrcIPFrom); - COPY_NEG_SIGN(rule->p.espHdrFilter.ipHdr.dataDstIPTo, - rule->p.espHdrFilter.ipHdr.dataDstIPFrom); + copy_neg_sign(&rule->p.espHdrFilter.ipHdr.dataSrcIPMask, + &rule->p.espHdrFilter.ipHdr.dataSrcIPAddr); + copy_neg_sign(&rule->p.espHdrFilter.ipHdr.dataDstIPMask, + &rule->p.espHdrFilter.ipHdr.dataDstIPAddr); + copy_neg_sign(&rule->p.espHdrFilter.ipHdr.dataSrcIPTo, + &rule->p.espHdrFilter.ipHdr.dataSrcIPFrom); + copy_neg_sign(&rule->p.espHdrFilter.ipHdr.dataDstIPTo, + &rule->p.espHdrFilter.ipHdr.dataDstIPFrom); virNWFilterRuleDefFixupIPSet(&rule->p.espHdrFilter.ipHdr); break; case VIR_NWFILTER_RULE_PROTOCOL_AH: case VIR_NWFILTER_RULE_PROTOCOL_AHoIPV6: - COPY_NEG_SIGN(rule->p.ahHdrFilter.ipHdr.dataSrcIPMask, - rule->p.ahHdrFilter.ipHdr.dataSrcIPAddr); - COPY_NEG_SIGN(rule->p.ahHdrFilter.ipHdr.dataDstIPMask, - rule->p.ahHdrFilter.ipHdr.dataDstIPAddr); - COPY_NEG_SIGN(rule->p.ahHdrFilter.ipHdr.dataSrcIPTo, - rule->p.ahHdrFilter.ipHdr.dataSrcIPFrom); - COPY_NEG_SIGN(rule->p.ahHdrFilter.ipHdr.dataDstIPTo, - rule->p.ahHdrFilter.ipHdr.dataDstIPFrom); + copy_neg_sign(&rule->p.ahHdrFilter.ipHdr.dataSrcIPMask, + &rule->p.ahHdrFilter.ipHdr.dataSrcIPAddr); + copy_neg_sign(&rule->p.ahHdrFilter.ipHdr.dataDstIPMask, + &rule->p.ahHdrFilter.ipHdr.dataDstIPAddr); + copy_neg_sign(&rule->p.ahHdrFilter.ipHdr.dataSrcIPTo, + &rule->p.ahHdrFilter.ipHdr.dataSrcIPFrom); + copy_neg_sign(&rule->p.ahHdrFilter.ipHdr.dataDstIPTo, + &rule->p.ahHdrFilter.ipHdr.dataDstIPFrom); virNWFilterRuleDefFixupIPSet(&rule->p.ahHdrFilter.ipHdr); break; case VIR_NWFILTER_RULE_PROTOCOL_SCTP: case VIR_NWFILTER_RULE_PROTOCOL_SCTPoIPV6: - COPY_NEG_SIGN(rule->p.sctpHdrFilter.ipHdr.dataSrcIPMask, - rule->p.sctpHdrFilter.ipHdr.dataSrcIPAddr); - COPY_NEG_SIGN(rule->p.sctpHdrFilter.ipHdr.dataDstIPMask, - rule->p.sctpHdrFilter.ipHdr.dataDstIPAddr); - COPY_NEG_SIGN(rule->p.sctpHdrFilter.ipHdr.dataSrcIPTo, - rule->p.sctpHdrFilter.ipHdr.dataSrcIPFrom); - COPY_NEG_SIGN(rule->p.sctpHdrFilter.ipHdr.dataDstIPTo, - rule->p.sctpHdrFilter.ipHdr.dataDstIPFrom); - COPY_NEG_SIGN(rule->p.sctpHdrFilter.portData.dataSrcPortEnd, - rule->p.sctpHdrFilter.portData.dataSrcPortStart); - COPY_NEG_SIGN(rule->p.sctpHdrFilter.portData.dataDstPortStart, - rule->p.sctpHdrFilter.portData.dataSrcPortStart); - COPY_NEG_SIGN(rule->p.sctpHdrFilter.portData.dataDstPortEnd, - rule->p.sctpHdrFilter.portData.dataSrcPortStart); + copy_neg_sign(&rule->p.sctpHdrFilter.ipHdr.dataSrcIPMask, + &rule->p.sctpHdrFilter.ipHdr.dataSrcIPAddr); + copy_neg_sign(&rule->p.sctpHdrFilter.ipHdr.dataDstIPMask, + &rule->p.sctpHdrFilter.ipHdr.dataDstIPAddr); + copy_neg_sign(&rule->p.sctpHdrFilter.ipHdr.dataSrcIPTo, + &rule->p.sctpHdrFilter.ipHdr.dataSrcIPFrom); + copy_neg_sign(&rule->p.sctpHdrFilter.ipHdr.dataDstIPTo, + &rule->p.sctpHdrFilter.ipHdr.dataDstIPFrom); + copy_neg_sign(&rule->p.sctpHdrFilter.portData.dataSrcPortEnd, + &rule->p.sctpHdrFilter.portData.dataSrcPortStart); + copy_neg_sign(&rule->p.sctpHdrFilter.portData.dataDstPortStart, + &rule->p.sctpHdrFilter.portData.dataSrcPortStart); + copy_neg_sign(&rule->p.sctpHdrFilter.portData.dataDstPortEnd, + &rule->p.sctpHdrFilter.portData.dataSrcPortStart); virNWFilterRuleDefFixupIPSet(&rule->p.sctpHdrFilter.ipHdr); break; case VIR_NWFILTER_RULE_PROTOCOL_ICMP: case VIR_NWFILTER_RULE_PROTOCOL_ICMPV6: - COPY_NEG_SIGN(rule->p.icmpHdrFilter.ipHdr.dataSrcIPMask, - rule->p.icmpHdrFilter.ipHdr.dataSrcIPAddr); - COPY_NEG_SIGN(rule->p.icmpHdrFilter.ipHdr.dataDstIPMask, - rule->p.icmpHdrFilter.ipHdr.dataDstIPAddr); - COPY_NEG_SIGN(rule->p.icmpHdrFilter.ipHdr.dataSrcIPTo, - rule->p.icmpHdrFilter.ipHdr.dataSrcIPFrom); - COPY_NEG_SIGN(rule->p.icmpHdrFilter.ipHdr.dataDstIPTo, - rule->p.icmpHdrFilter.ipHdr.dataDstIPFrom); - COPY_NEG_SIGN(rule->p.icmpHdrFilter.dataICMPCode, - rule->p.icmpHdrFilter.dataICMPType); + copy_neg_sign(&rule->p.icmpHdrFilter.ipHdr.dataSrcIPMask, + &rule->p.icmpHdrFilter.ipHdr.dataSrcIPAddr); + copy_neg_sign(&rule->p.icmpHdrFilter.ipHdr.dataDstIPMask, + &rule->p.icmpHdrFilter.ipHdr.dataDstIPAddr); + copy_neg_sign(&rule->p.icmpHdrFilter.ipHdr.dataSrcIPTo, + &rule->p.icmpHdrFilter.ipHdr.dataSrcIPFrom); + copy_neg_sign(&rule->p.icmpHdrFilter.ipHdr.dataDstIPTo, + &rule->p.icmpHdrFilter.ipHdr.dataDstIPFrom); + copy_neg_sign(&rule->p.icmpHdrFilter.dataICMPCode, + &rule->p.icmpHdrFilter.dataICMPType); virNWFilterRuleDefFixupIPSet(&rule->p.icmpHdrFilter.ipHdr); break; case VIR_NWFILTER_RULE_PROTOCOL_ALL: case VIR_NWFILTER_RULE_PROTOCOL_ALLoIPV6: - COPY_NEG_SIGN(rule->p.allHdrFilter.ipHdr.dataSrcIPMask, - rule->p.allHdrFilter.ipHdr.dataSrcIPAddr); - COPY_NEG_SIGN(rule->p.allHdrFilter.ipHdr.dataDstIPMask, - rule->p.allHdrFilter.ipHdr.dataDstIPAddr); - COPY_NEG_SIGN(rule->p.allHdrFilter.ipHdr.dataSrcIPTo, - rule->p.allHdrFilter.ipHdr.dataSrcIPFrom); - COPY_NEG_SIGN(rule->p.allHdrFilter.ipHdr.dataDstIPTo, - rule->p.allHdrFilter.ipHdr.dataDstIPFrom); + copy_neg_sign(&rule->p.allHdrFilter.ipHdr.dataSrcIPMask, + &rule->p.allHdrFilter.ipHdr.dataSrcIPAddr); + copy_neg_sign(&rule->p.allHdrFilter.ipHdr.dataDstIPMask, + &rule->p.allHdrFilter.ipHdr.dataDstIPAddr); + copy_neg_sign(&rule->p.allHdrFilter.ipHdr.dataSrcIPTo, + &rule->p.allHdrFilter.ipHdr.dataSrcIPFrom); + copy_neg_sign(&rule->p.allHdrFilter.ipHdr.dataDstIPTo, + &rule->p.allHdrFilter.ipHdr.dataDstIPFrom); break; case VIR_NWFILTER_RULE_PROTOCOL_IGMP: - COPY_NEG_SIGN(rule->p.igmpHdrFilter.ipHdr.dataSrcIPMask, - rule->p.igmpHdrFilter.ipHdr.dataSrcIPAddr); - COPY_NEG_SIGN(rule->p.igmpHdrFilter.ipHdr.dataDstIPMask, - rule->p.igmpHdrFilter.ipHdr.dataDstIPAddr); - COPY_NEG_SIGN(rule->p.igmpHdrFilter.ipHdr.dataSrcIPTo, - rule->p.igmpHdrFilter.ipHdr.dataSrcIPFrom); - COPY_NEG_SIGN(rule->p.igmpHdrFilter.ipHdr.dataDstIPTo, - rule->p.igmpHdrFilter.ipHdr.dataDstIPFrom); + copy_neg_sign(&rule->p.igmpHdrFilter.ipHdr.dataSrcIPMask, + &rule->p.igmpHdrFilter.ipHdr.dataSrcIPAddr); + copy_neg_sign(&rule->p.igmpHdrFilter.ipHdr.dataDstIPMask, + &rule->p.igmpHdrFilter.ipHdr.dataDstIPAddr); + copy_neg_sign(&rule->p.igmpHdrFilter.ipHdr.dataSrcIPTo, + &rule->p.igmpHdrFilter.ipHdr.dataSrcIPFrom); + copy_neg_sign(&rule->p.igmpHdrFilter.ipHdr.dataDstIPTo, + &rule->p.igmpHdrFilter.ipHdr.dataDstIPFrom); virNWFilterRuleDefFixupIPSet(&rule->p.igmpHdrFilter.ipHdr); break; case VIR_NWFILTER_RULE_PROTOCOL_LAST: break; } -#undef COPY_NEG_SIGN } -- 2.31.1

On Mon, Sep 20, 2021 at 19:21:28 +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/nwfilter_conf.c | 284 ++++++++++++++++++++------------------- 1 file changed, 143 insertions(+), 141 deletions(-)
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index a3109962af..4c4e31d5cd 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2187,71 +2187,74 @@ virNWFilterRuleValidate(virNWFilterRuleDef *rule)
static void -virNWFilterRuleDefFixup(virNWFilterRuleDef *rule) +copy_neg_sign(nwItemDesc *to, const nwItemDesc *from)
Please use a name conformant to our coding style for this function before pushing.

On 9/20/21 7:21 PM, Tim Wiederhake wrote:
This is an alternative to https://listman.redhat.com/archives/libvir-list/2021-September/msg00522.html.
When libvirt is build: * with sanitizers enabled, * buildtype explicitly set to "debug", * on clang,
the build fails with:
../src/conf/nwfilter_conf.c:2190:1: error: stack frame size of 10616 bytes in function 'virNWFilterRuleDefFixup' [-Werror,-Wframe-larger-than=] virNWFilterRuleDefFixup(virNWFilterRuleDef *rule) ^ 1 error generated.
../src/conf/domain_conf.c:19514:1: error: stack frame size of 8312 bytes in function 'virDomainDefParseXML' [-Werror,-Wframe-larger-than=] virDomainDefParseXML(xmlXPathContextPtr ctxt, ^ 1 error generated.
Note that this does not happen when "-Dbuildtype" is not specified, even though "debug" is the default build type.
The patches in this series happen to make these errors go away.
Regards, Tim
Tim Wiederhake (2): virDomainDefParseXML: Use automatic memory management virNWFilterRuleDefFixup: Replace macro with function
src/conf/domain_conf.c | 208 ++++++++++++++-------------- src/conf/nwfilter_conf.c | 284 ++++++++++++++++++++------------------- 2 files changed, 245 insertions(+), 247 deletions(-)
Frankly, I don't understand how either of patches can size the frame down, but they make sense regardless. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

On Thu, Sep 23, 2021 at 09:56:01AM +0200, Michal Prívozník wrote:
On 9/20/21 7:21 PM, Tim Wiederhake wrote:
This is an alternative to https://listman.redhat.com/archives/libvir-list/2021-September/msg00522.html.
When libvirt is build: * with sanitizers enabled, * buildtype explicitly set to "debug", * on clang,
the build fails with:
../src/conf/nwfilter_conf.c:2190:1: error: stack frame size of 10616 bytes in function 'virNWFilterRuleDefFixup' [-Werror,-Wframe-larger-than=] virNWFilterRuleDefFixup(virNWFilterRuleDef *rule) ^ 1 error generated.
../src/conf/domain_conf.c:19514:1: error: stack frame size of 8312 bytes in function 'virDomainDefParseXML' [-Werror,-Wframe-larger-than=] virDomainDefParseXML(xmlXPathContextPtr ctxt, ^ 1 error generated.
Note that this does not happen when "-Dbuildtype" is not specified, even though "debug" is the default build type.
The patches in this series happen to make these errors go away.
Regards, Tim
Tim Wiederhake (2): virDomainDefParseXML: Use automatic memory management virNWFilterRuleDefFixup: Replace macro with function
src/conf/domain_conf.c | 208 ++++++++++++++-------------- src/conf/nwfilter_conf.c | 284 ++++++++++++++++++++------------------- 2 files changed, 245 insertions(+), 247 deletions(-)
Frankly, I don't understand how either of patches can size the frame down, but they make sense regardless.
The first patch is fine, but I don't think the second is. It introduces countless more pointless fnuction calls when a macro suffices. We just need to increase the stack frame check limit for instrumented builds. Enforcing stack frame only makes sense on normal builds when stack usage is determinstic Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (4)
-
Daniel P. Berrangé
-
Michal Prívozník
-
Peter Krempa
-
Tim Wiederhake