Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- src/libxl/libxl_domain.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index a3f362a0c8..c96aeab04b 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -1261,7 +1261,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, uint32_t restore_ver LIBXL_DOMSTART_RESTORE_VER_ATTR) { libxl_domain_config d_config; - virDomainDefPtr def = NULL; + g_autoptr(virDomainDef) def = NULL; virObjectEventPtr event = NULL; libxlSavefileHeader hdr; int ret = -1; @@ -1520,7 +1520,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver, VIR_FREE(config_json); VIR_FREE(dom_xml); VIR_FREE(managed_save_path); - virDomainDefFree(def); VIR_FORCE_CLOSE(managed_save_fd); return ret; } -- 2.26.2

It is only used once, so it's pointless to free it both in the code and in the cleanup section. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- src/libxl/libxl_domain.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index c96aeab04b..034be2ddd7 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -1267,7 +1267,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, int ret = -1; uint32_t domid = 0; char *dom_xml = NULL; - char *managed_save_path = NULL; + g_autofree char *managed_save_path = NULL; int managed_save_fd = -1; libxlDomainObjPrivatePtr priv = vm->privateData; g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver); @@ -1323,7 +1323,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver, vm->hasManagedSave = false; } - VIR_FREE(managed_save_path); } if (virDomainObjSetDefTransient(driver->xmlopt, vm, NULL) < 0) @@ -1519,7 +1518,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver, libxl_domain_config_dispose(&d_config); VIR_FREE(config_json); VIR_FREE(dom_xml); - VIR_FREE(managed_save_path); VIR_FORCE_CLOSE(managed_save_fd); return ret; } -- 2.26.2

Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- src/libxl/libxl_domain.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 034be2ddd7..b49ca83c10 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -1266,16 +1266,16 @@ libxlDomainStart(libxlDriverPrivatePtr driver, libxlSavefileHeader hdr; int ret = -1; uint32_t domid = 0; - char *dom_xml = NULL; + g_autofree char *dom_xml = NULL; g_autofree char *managed_save_path = NULL; - int managed_save_fd = -1; + VIR_AUTOCLOSE managed_save_fd = -1; libxlDomainObjPrivatePtr priv = vm->privateData; g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver); virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; libxl_asyncprogress_how aop_console_how; libxl_domain_restore_params params; unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI; - char *config_json = NULL; + g_autofree char *config_json = NULL; #ifdef LIBXL_HAVE_PVUSB hostdev_flags |= VIR_HOSTDEV_SP_USB; @@ -1516,9 +1516,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver, cleanup: libxl_domain_config_dispose(&d_config); - VIR_FREE(config_json); - VIR_FREE(dom_xml); - VIR_FORCE_CLOSE(managed_save_fd); return ret; } -- 2.26.2

clang reports: stack frame size of 2152 bytes in function 'libxlDomainStart' This is mostly due to the d_config variable: sizeof(libxl_domain_config) = 1232 Use g_new0 to allocate it and bring the frame size down. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- src/libxl/libxl_domain.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index b49ca83c10..6336c87746 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -1260,7 +1260,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, int restore_fd, uint32_t restore_ver LIBXL_DOMSTART_RESTORE_VER_ATTR) { - libxl_domain_config d_config; + g_autofree libxl_domain_config *d_config = NULL; g_autoptr(virDomainDef) def = NULL; virObjectEventPtr event = NULL; libxlSavefileHeader hdr; @@ -1281,7 +1281,9 @@ libxlDomainStart(libxlDriverPrivatePtr driver, hostdev_flags |= VIR_HOSTDEV_SP_USB; #endif - libxl_domain_config_init(&d_config); + d_config = g_new0(libxl_domain_config, 1); + + libxl_domain_config_init(d_config); /* If there is a managed saved state restore it instead of starting * from scratch. The old state is removed once the restoring succeeded. */ @@ -1356,10 +1358,10 @@ libxlDomainStart(libxlDriverPrivatePtr driver, goto cleanup_dom; if (libxlBuildDomainConfig(driver->reservedGraphicsPorts, vm->def, - cfg, &d_config) < 0) + cfg, d_config) < 0) goto cleanup_dom; - if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, &d_config) < 0) + if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, d_config) < 0) goto cleanup_dom; if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME, @@ -1399,14 +1401,14 @@ libxlDomainStart(libxlDriverPrivatePtr driver, aop_console_how.for_callback = vm; aop_console_how.callback = libxlConsoleCallback; if (restore_fd < 0) { - ret = libxl_domain_create_new(cfg->ctx, &d_config, + ret = libxl_domain_create_new(cfg->ctx, d_config, &domid, NULL, &aop_console_how); } else { libxl_domain_restore_params_init(&params); #ifdef LIBXL_HAVE_SRM_V2 params.stream_version = restore_ver; #endif - ret = libxl_domain_create_restore(cfg->ctx, &d_config, &domid, + ret = libxl_domain_create_restore(cfg->ctx, d_config, &domid, restore_fd, &params, NULL, &aop_console_how); libxl_domain_restore_params_dispose(&params); @@ -1417,11 +1419,11 @@ libxlDomainStart(libxlDriverPrivatePtr driver, if (restore_fd < 0) virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to create new domain '%s'"), - d_config.c_info.name); + d_config->c_info.name); else virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to restore domain '%s'"), - d_config.c_info.name); + d_config->c_info.name); goto cleanup_dom; } @@ -1430,7 +1432,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, * be cleaned up if there are any subsequent failures. */ vm->def->id = domid; - config_json = libxl_domain_config_to_json(cfg->ctx, &d_config); + config_json = libxl_domain_config_to_json(cfg->ctx, d_config); libxlLoggerOpenFile(cfg->logger, domid, vm->def->name, config_json); @@ -1445,7 +1447,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, if (libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW)) goto destroy_dom; - libxlDomainCreateIfaceNames(vm->def, &d_config); + libxlDomainCreateIfaceNames(vm->def, d_config); libxlDomainUpdateDiskParams(vm->def, cfg->ctx); #ifdef LIBXL_HAVE_DEVICE_CHANNEL @@ -1515,7 +1517,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, libxlDomainCleanup(driver, vm); cleanup: - libxl_domain_config_dispose(&d_config); + libxl_domain_config_dispose(d_config); return ret; } -- 2.26.2

Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- src/esx/esx_driver.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index e82e5ed835..0798493296 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -813,7 +813,7 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, virDrvOpenStatus result = VIR_DRV_OPEN_ERROR; esxPrivate *priv = NULL; char *potentialVCenterIPAddress = NULL; - char vCenterIPAddress[NI_MAXHOST] = ""; + g_autofree char *vCenterIPAddress = g_new0(char, NI_MAXHOST); virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); @@ -875,8 +875,9 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, goto cleanup; } - if (virStrcpyStatic(vCenterIPAddress, - potentialVCenterIPAddress) < 0) { + if (virStrcpy(vCenterIPAddress, + potentialVCenterIPAddress, + NI_MAXHOST) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("vCenter IP address %s too big for destination"), potentialVCenterIPAddress); -- 2.26.2

Lower the stack frame by using allocated buffers. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- src/lxc/lxc_process.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 16969dbf33..bf050416c0 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1185,7 +1185,6 @@ int virLXCProcessStart(virConnectPtr conn, VIR_AUTOSTRINGLIST veths = NULL; int handshakefds[2] = { -1, -1 }; off_t pos = -1; - char ebuf[1024]; g_autofree char *timestamp = NULL; int nsInheritFDs[VIR_LXC_DOMAIN_NAMESPACE_LAST]; virCommandPtr cmd = NULL; @@ -1402,13 +1401,14 @@ int virLXCProcessStart(virConnectPtr conn, goto cleanup; if (status != 0) { + g_autofree char *ebuf = g_new0(char, BUFSIZ); if (virLXCProcessReadLogOutput(vm, logfile, pos, ebuf, - sizeof(ebuf)) <= 0) { + BUFSIZ) <= 0) { if (WIFEXITED(status)) - g_snprintf(ebuf, sizeof(ebuf), _("unexpected exit status %d"), + g_snprintf(ebuf, BUFSIZ, _("unexpected exit status %d"), WEXITSTATUS(status)); else - g_snprintf(ebuf, sizeof(ebuf), "%s", _("terminated abnormally")); + g_snprintf(ebuf, BUFSIZ, "%s", _("terminated abnormally")); } virReportError(VIR_ERR_INTERNAL_ERROR, _("guest failed to start: %s"), ebuf); @@ -1417,7 +1417,8 @@ int virLXCProcessStart(virConnectPtr conn, /* It has started running, so get its pid */ if ((r = virPidFileReadPath(pidfile, &vm->pid)) < 0) { - if (virLXCProcessReadLogOutput(vm, logfile, pos, ebuf, sizeof(ebuf)) > 0) + g_autofree char *ebuf = g_new0(char, BUFSIZ); + if (virLXCProcessReadLogOutput(vm, logfile, pos, ebuf, BUFSIZ) > 0) virReportError(VIR_ERR_INTERNAL_ERROR, _("guest failed to start: %s"), ebuf); else @@ -1454,9 +1455,9 @@ int virLXCProcessStart(virConnectPtr conn, driver->inhibitCallback(true, driver->inhibitOpaque); if (lxcContainerWaitForContinue(handshakefds[0]) < 0) { - char out[1024]; + g_autofree char *out = g_new0(char, BUFSIZ); - if (!(virLXCProcessReadLogOutput(vm, logfile, pos, out, 1024) < 0)) { + if (!(virLXCProcessReadLogOutput(vm, logfile, pos, out, BUFSIZ) < 0)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("guest failed to start: %s"), out); } @@ -1486,10 +1487,12 @@ int virLXCProcessStart(virConnectPtr conn, /* And we can get the first monitor connection now too */ if (!(priv->monitor = virLXCProcessConnectMonitor(driver, vm))) { + g_autofree char *ebuf = g_new0(char, BUFSIZ); + /* Intentionally overwrite the real monitor error message, * since a better one is almost always found in the logs */ - if (virLXCProcessReadLogOutput(vm, logfile, pos, ebuf, sizeof(ebuf)) > 0) { + if (virLXCProcessReadLogOutput(vm, logfile, pos, ebuf, BUFSIZ) > 0) { virResetLastError(); virReportError(VIR_ERR_INTERNAL_ERROR, _("guest failed to start: %s"), ebuf); -- 2.26.2

This number is the closest multiple of 100000000 above the largest frame value reported by clang in the current codebase. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- meson.build | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/meson.build b/meson.build index a5ce8e17a8..8bef701f67 100644 --- a/meson.build +++ b/meson.build @@ -419,10 +419,9 @@ cc_flags += [ # but need to rewrite various areas of code first '-Wno-format-truncation', - # This should be < 256 really. Currently we're down to 4096, - # but using 1024 bytes sized buffers (mostly for virStrerror) - # stops us from going down further - '-Wframe-larger-than=4096', + # This should be < 256 really. + # Using 1024 bytes sized buffers stops us from going down further + '-Wframe-larger-than=1792', # extra special flags '-fexceptions', -- 2.26.2

On a Monday in 2020, Daniel P. Berrangé wrote: 0x100 for short Yeah, 256 is impossible to achieve. Especially since clang seems more conservative with the numbers it prints. I merely deleted the parts of the comment with outdated information. Jano