From nobody Thu Dec 18 14:17:32 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) client-ip=8.43.85.245; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=pass(p=reject dis=none) header.from=lists.libvirt.org ARC-Seal: i=1; a=rsa-sha256; t=1743773349; cv=none; d=zohomail.com; s=zohoarc; b=NRWBOOvAbqapn9gytTHJTLFeofEFgykbDc26f+Y9MB9CEH+v0VoDXsVkL2MjYlctJGKxyt+86pEIapyAvG6E2Td7lsM8zN1rHMk8PeP+DdvFhlzmmI1X9CUPky5mpQntWlumFqmzOAnj4ptTwWGP/3UhXzDcAgUIA8/c9zER/rU= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1743773349; h=Content-Type:Content-Transfer-Encoding:Date:Date:From:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:Reply-To:References:Subject:Subject:To:To:Message-Id:Cc; bh=TFBNgw0f24Um14UOmS/N4+Y3pXPg11Rr7HU1xQ2D4Lg=; b=BceJ9MHrNOmDluN/uehiSLYyxbmocbNKhF4mxRG0jz//lVkAvhwWO7PoA9HuneYRsiuc3FI7+54ExpU6KzrkDKx/d6yaiRrDQhfjJGdlzp/hc2m09TENkq8/bBTG5+Kve0nRbCHFJ9JT4ucTgU86Wt1yQy7KTe4xnvEeeb3KIhE= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=pass header.from= (p=reject dis=none) Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 1743773349204929.7987039923744; Fri, 4 Apr 2025 06:29:09 -0700 (PDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 95BDC149D; Fri, 4 Apr 2025 09:29:08 -0400 (EDT) Received: from lists.libvirt.org (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id 6391915D0; Fri, 4 Apr 2025 09:28:13 -0400 (EDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 8EE1AB90; Fri, 4 Apr 2025 09:28:07 -0400 (EDT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with ESMTPS id EF25611AA for ; Fri, 4 Apr 2025 09:28:06 -0400 (EDT) Received: from mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-42-jY8qzxdKNIea01OecOPBNA-1; Fri, 04 Apr 2025 09:28:05 -0400 Received: from mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.15]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id C9E321955DAF for ; Fri, 4 Apr 2025 13:28:04 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.3.236]) by mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 207551955BC2 for ; Fri, 4 Apr 2025 13:28:03 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on lists.libvirt.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED, MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL,RCVD_IN_VALIDITY_RPBL_BLOCKED, RCVD_IN_VALIDITY_SAFE_BLOCKED,SPF_HELO_NONE autolearn=unavailable autolearn_force=no version=3.4.4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1743773286; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=HEQU+q2cmGRu5P15ODcWNSuK52KDShmAqO2VLsbwhXc=; b=N1+MQdWTqgFQmCdCWkHj7D+NMxp+XobdC7DlgTwrMyioBebV6mG+xmztMdXHvMbBy9kUk1 K5sVjkdwqLRstUm7JncX9rsNbblUBCB6RrKHtwDFnB+bdDey9vA7waleUnVslnSbTqzY7x 5ELLH1GgXdaGhEYpfWXUiMwf6G9plzY= X-MC-Unique: jY8qzxdKNIea01OecOPBNA-1 X-Mimecast-MFC-AGG-ID: jY8qzxdKNIea01OecOPBNA_1743773284 To: devel@lists.libvirt.org Subject: [PATCH 1/8] remote_driver: Move URI arg extraction into a separate function Date: Fri, 4 Apr 2025 15:27:53 +0200 Message-ID: <90f3dc9234023976914ecefe8571bd83e59b3874.1743772965.git.mprivozn@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.15 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: N1kQEBk6x6_jSNXzlwJbiZH_z7vrJV8CDxpLvl-J6nI_1743773284 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Message-ID-Hash: LA5CV7IHCODZL6LJ5DJLGLGJINAASI67 X-Message-ID-Hash: LA5CV7IHCODZL6LJ5DJLGLGJINAASI67 X-MailFrom: mprivozn@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; header-match-devel.lists.libvirt.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header X-Mailman-Version: 3.2.2 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: From: Michal Privoznik via Devel Reply-To: Michal Privoznik X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZM-MESSAGEID: 1743773349987019000 Content-Type: text/plain; charset="utf-8"; x-default="true" From: Michal Privoznik There's a problem with glib: what we might consider functions are in fact macros and to make things worse - they do declare local variables. For instance here's the declaration of g_clear_pointer() macro: #define g_clear_pointer(pp, destroy) \ G_STMT_START \ { \ G_STATIC_ASSERT (sizeof *(pp) =3D=3D sizeof (gpointer)); \ glib_typeof ((pp)) _pp =3D (pp); \ glib_typeof (*(pp)) _ptr =3D *_pp; \ *_pp =3D NULL; \ if (_ptr) \ (destroy) (_ptr); \ } \ G_STMT_END \ Now, as of v6.2.0-rc1~267 our VIR_FREE() macro is in fact a redeclaration of g_clear_pointer(). Thus, calling VIR_FREE() increases stack size! Ideally, this wouldn't be a problem, because those variables (_pp, _ptr) live in their own block. And clever compiler can just reuse space created for one block. But then there's clang where we are hitting this exact problem in functions like doRemoteOpen() where either g_clear_pointer() is called directly, or there are macros like EXTRACT_URI_ARG_STR() which hide the call away. That's why despite our previous efforts decreasing stack size we still needed v9.8.0-rc1~208. Well, moving URI argument extraction (those calls to EXTRACT_URI_ARG_* macros) into a separate function helps us decrease stack size from 2296 bytes to 1320. Even after this there are still more possibilities for improvements, but those will be addressed in future commits. Signed-off-by: Michal Privoznik --- src/remote/remote_driver.c | 113 +++++++++++++++++++++++++++---------- 1 file changed, 82 insertions(+), 31 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 745cd34f83..681594e406 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -758,13 +758,74 @@ remoteConnectFormatURI(virURI *uri, virReportError(VIR_ERR_INVALID_ARG, \ _("Failed to parse value of URI component %1$s"= ), \ var->name); \ - goto error; \ + return -1; \ } \ ARG_VAR =3D tmp =3D=3D 0; \ var->ignore =3D 1; \ continue; \ } =20 +static int +doRemoteOpenExtractURIArgs(virConnectPtr conn, + char **name, + char **command, + char **sockname, + char **authtype, + char **sshauth, + char **netcat, + char **keyfile, + char **pkipath, + char **knownHosts, + char **knownHostsVerify, + char **tls_priority, + char **mode_str, + char **proxy_str, +#ifndef WIN32 + bool *tty, +#endif + bool *sanity, + bool *verify) +{ + size_t i; + + for (i =3D 0; i < conn->uri->paramsCount; i++) { + virURIParam *var =3D &conn->uri->params[i]; + + EXTRACT_URI_ARG_STR("name", *name); + EXTRACT_URI_ARG_STR("command", *command); + EXTRACT_URI_ARG_STR("socket", *sockname); + EXTRACT_URI_ARG_STR("auth", *authtype); + EXTRACT_URI_ARG_STR("sshauth", *sshauth); + EXTRACT_URI_ARG_STR("netcat", *netcat); + EXTRACT_URI_ARG_STR("keyfile", *keyfile); + EXTRACT_URI_ARG_STR("pkipath", *pkipath); + EXTRACT_URI_ARG_STR("known_hosts", *knownHosts); + EXTRACT_URI_ARG_STR("known_hosts_verify", *knownHostsVerify); + EXTRACT_URI_ARG_STR("tls_priority", *tls_priority); + EXTRACT_URI_ARG_STR("mode", *mode_str); + EXTRACT_URI_ARG_STR("proxy", *proxy_str); + EXTRACT_URI_ARG_BOOL("no_sanity", *sanity); + EXTRACT_URI_ARG_BOOL("no_verify", *verify); +#ifndef WIN32 + EXTRACT_URI_ARG_BOOL("no_tty", *tty); +#endif + + if (STRCASEEQ(var->name, "authfile")) { + /* Strip this param, used by virauth.c */ + var->ignore =3D 1; + continue; + } + + VIR_DEBUG("passing through variable '%s' ('%s') to remote end", + var->name, var->value); + } + + return 0; +} + +#undef EXTRACT_URI_ARG_STR +#undef EXTRACT_URI_ARG_BOOL + =20 /* * URIs that this driver needs to handle: @@ -818,7 +879,6 @@ doRemoteOpen(virConnectPtr conn, bool tty =3D true; #endif int mode; - size_t i; int proxy; =20 /* We handle *ALL* URIs here. The caller has rejected any @@ -844,35 +904,28 @@ doRemoteOpen(virConnectPtr conn, * although that won't be the case for now). */ if (conn->uri) { - for (i =3D 0; i < conn->uri->paramsCount; i++) { - virURIParam *var =3D &conn->uri->params[i]; - EXTRACT_URI_ARG_STR("name", name); - EXTRACT_URI_ARG_STR("command", command); - EXTRACT_URI_ARG_STR("socket", sockname); - EXTRACT_URI_ARG_STR("auth", authtype); - EXTRACT_URI_ARG_STR("sshauth", sshauth); - EXTRACT_URI_ARG_STR("netcat", netcat); - EXTRACT_URI_ARG_STR("keyfile", keyfile); - EXTRACT_URI_ARG_STR("pkipath", pkipath); - EXTRACT_URI_ARG_STR("known_hosts", knownHosts); - EXTRACT_URI_ARG_STR("known_hosts_verify", knownHostsVerify); - EXTRACT_URI_ARG_STR("tls_priority", tls_priority); - EXTRACT_URI_ARG_STR("mode", mode_str); - EXTRACT_URI_ARG_STR("proxy", proxy_str); - EXTRACT_URI_ARG_BOOL("no_sanity", sanity); - EXTRACT_URI_ARG_BOOL("no_verify", verify); + /* This really needs to be a separate function to keep + * the stack size at sane levels. */ + if (doRemoteOpenExtractURIArgs(conn, + &name, + &command, + &sockname, + &authtype, + &sshauth, + &netcat, + &keyfile, + &pkipath, + &knownHosts, + &knownHostsVerify, + &tls_priority, + &mode_str, + &proxy_str, #ifndef WIN32 - EXTRACT_URI_ARG_BOOL("no_tty", tty); + &tty, #endif - - if (STRCASEEQ(var->name, "authfile")) { - /* Strip this param, used by virauth.c */ - var->ignore =3D 1; - continue; - } - - VIR_DEBUG("passing through variable '%s' ('%s') to remote end", - var->name, var->value); + &sanity, + &verify) < 0) { + goto error; } =20 /* Construct the original name. */ @@ -1248,8 +1301,6 @@ doRemoteOpen(virConnectPtr conn, VIR_FREE(priv->hostname); return VIR_DRV_OPEN_ERROR; } -#undef EXTRACT_URI_ARG_STR -#undef EXTRACT_URI_ARG_BOOL =20 static struct private_data * remoteAllocPrivateData(void) --=20 2.49.0