[libvirt PATCH 0/8] build: lower maximum frame size to 1792
Ján Tomko (8): libxl: libxlDomainStart: use g_autoptr for virDomainDef libxl: libxlDomainStart: autofree managed_save_path libxl: libxlDomainStart: use g_auto more libxl: allocate d_config esx: esxConnectOpen: use allocated buffer lxc: virLXCProcessStart: use allocated buffers remote: allocate virDomainDef for ACL check build: lower maximum frame size to 1792 meson.build | 7 +++-- src/esx/esx_driver.c | 7 ++--- src/libxl/libxl_domain.c | 40 +++++++++++++---------------- src/lxc/lxc_process.c | 19 ++++++++------ src/remote/remote_daemon_dispatch.c | 19 +++++++------- 5 files changed, 46 insertions(+), 46 deletions(-) -- 2.26.2
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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
On 10/4/20 4:21 PM, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libxl/libxl_domain.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Jim Fehlig <jfehlig@suse.com> Regards, Jim
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; }
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@redhat.com> --- 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
On 10/4/20 4:21 PM, Ján Tomko wrote:
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@redhat.com> --- src/libxl/libxl_domain.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Jim Fehlig <jfehlig@suse.com> Regards, Jim
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; }
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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
On 10/4/20 4:21 PM, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libxl/libxl_domain.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
Reviewed-by: Jim Fehlig <jfehlig@suse.com> Regards, Jim
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; }
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@redhat.com> --- 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(¶ms); #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, ¶ms, NULL, &aop_console_how); libxl_domain_restore_params_dispose(¶ms); @@ -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
On 10/4/20 4:21 PM, Ján Tomko wrote:
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@redhat.com> --- src/libxl/libxl_domain.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-)
Reviewed-by: Jim Fehlig <jfehlig@suse.com> Regards, Jim
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(¶ms); #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, ¶ms, NULL, &aop_console_how); libxl_domain_restore_params_dispose(¶ms); @@ -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; }
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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
On Monday, 5 October 2020 00:21:42 CEST Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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);
Hmm, this is not the only char[NI_MAXHOST] in this file, so I'm surprised only this one triggered the stack limit check. The NI_MAXHOST-limited buffer seems to be due to esxUtil_ResolveHostname(), which forces it due to its interface. I'll rewrite it to return a new string instead; consider it as a NACK for this patch. -- Pino Toscano
Lower the stack frame by using allocated buffers. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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
Use explicit g_free in the cleanup section, since we're not dealing with a proper virDomainDef, but just a shallow copy. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/remote/remote_daemon_dispatch.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index c187932a3c..fc90bd1251 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -158,25 +158,25 @@ static bool remoteRelayDomainEventCheckACL(virNetServerClientPtr client, virConnectPtr conn, virDomainPtr dom) { - virDomainDef def; + virDomainDefPtr def = g_new0(virDomainDef, 1); g_autoptr(virIdentity) identity = NULL; bool ret = false; /* For now, we just create a virDomainDef with enough contents to * satisfy what viraccessdriverpolkit.c references. This is a bit * fragile, but I don't know of anything better. */ - memset(&def, 0, sizeof(def)); - def.name = dom->name; - memcpy(def.uuid, dom->uuid, VIR_UUID_BUFLEN); + def->name = dom->name; + memcpy(def->uuid, dom->uuid, VIR_UUID_BUFLEN); if (!(identity = virNetServerClientGetIdentity(client))) goto cleanup; if (virIdentitySetCurrent(identity) < 0) goto cleanup; - ret = virConnectDomainEventRegisterAnyCheckACL(conn, &def); + ret = virConnectDomainEventRegisterAnyCheckACL(conn, def); cleanup: ignore_value(virIdentitySetCurrent(NULL)); + g_free(def); /* shallow free */ return ret; } @@ -288,24 +288,25 @@ static bool remoteRelayDomainQemuMonitorEventCheckACL(virNetServerClientPtr client, virConnectPtr conn, virDomainPtr dom) { - virDomainDef def; + virDomainDefPtr def = g_new0(virDomainDef, 1); g_autoptr(virIdentity) identity = NULL; bool ret = false; /* For now, we just create a virDomainDef with enough contents to * satisfy what viraccessdriverpolkit.c references. This is a bit * fragile, but I don't know of anything better. */ - def.name = dom->name; - memcpy(def.uuid, dom->uuid, VIR_UUID_BUFLEN); + def->name = dom->name; + memcpy(def->uuid, dom->uuid, VIR_UUID_BUFLEN); if (!(identity = virNetServerClientGetIdentity(client))) goto cleanup; if (virIdentitySetCurrent(identity) < 0) goto cleanup; - ret = virConnectDomainQemuMonitorEventRegisterCheckACL(conn, &def); + ret = virConnectDomainQemuMonitorEventRegisterCheckACL(conn, def); cleanup: ignore_value(virIdentitySetCurrent(NULL)); + g_free(def); /* shallow free */ return ret; } -- 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@redhat.com> --- 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 Mon, Oct 05, 2020 at 12:21:45AM +0200, Ján Tomko wrote:
This number is the closest multiple of 100000000
^^^ Huh ?
above the largest frame value reported by clang in the current codebase.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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.
I think we can drop this as I doubt it is worth trying to achieve. We've not knowingly had stack overflow in livirt in history.
+ # Using 1024 bytes sized buffers stops us from going down further + '-Wframe-larger-than=1792',
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 :|
On a Monday in 2020, Daniel P. Berrangé wrote:
On Mon, Oct 05, 2020 at 12:21:45AM +0200, Ján Tomko wrote:
This number is the closest multiple of 100000000
^^^ Huh ?
0x100 for short
above the largest frame value reported by clang in the current codebase.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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.
I think we can drop this as I doubt it is worth trying to achieve. We've not knowingly had stack overflow in livirt in history.
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
+ # Using 1024 bytes sized buffers stops us from going down further + '-Wframe-larger-than=1792',
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é -
Jim Fehlig -
Ján Tomko -
Pino Toscano