git: a5fee360b4d5 - stable/13 - bhyve: Cast away const when fetching a config nvlist

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Tue, 29 Nov 2022 17:40:24 UTC
The branch stable/13 has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=a5fee360b4d5fa0a5f6b1c9e556f50e887ceda45

commit a5fee360b4d5fa0a5f6b1c9e556f50e887ceda45
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2022-11-11 15:02:42 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2022-11-29 17:40:13 +0000

    bhyve: Cast away const when fetching a config nvlist
    
    Silence a warning from the compiler about "const" being discarded.  The
    warning is correct: nvlist values are supposed to be immutable.
    However, fixing this properly will require some contortions on behalf of
    consumers who look up a subtree of the config and modify it.  Per a
    discussion on freebsd-virtualization@, the solution will probably be to
    outright replace the use of nvlists for VM configuration, but until that
    happens let's document the problem and silence the warning.
    
    No functional change intended.
    
    MFC after:      2 weeks
    Reviewed by:    corvink, jhb
    Differential Revision:  https://reviews.freebsd.org/D37293
    
    (cherry picked from commit 719e307f80c724a39814557e35ac890c75dcd402)
---
 usr.sbin/bhyve/config.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/usr.sbin/bhyve/config.c b/usr.sbin/bhyve/config.c
index f398ea76fc58..e302f919adab 100644
--- a/usr.sbin/bhyve/config.c
+++ b/usr.sbin/bhyve/config.c
@@ -65,7 +65,17 @@ _lookup_config_node(nvlist_t *parent, const char *path, bool create)
 			break;
 		}
 		if (nvlist_exists_nvlist(nvl, name))
-			nvl = (nvlist_t *)nvlist_get_nvlist(nvl, name);
+			/*
+			 * XXX-MJ it is incorrect to cast away the const
+			 * qualifier like this since the contract with nvlist
+			 * says that values are immuatable, and some consumers
+			 * will indeed add nodes to the returned nvlist.  In
+			 * practice, however, it appears to be harmless with the
+			 * current nvlist implementation, so we just live with
+			 * it until the implementation is reworked.
+			 */
+			nvl = __DECONST(nvlist_t *,
+			    nvlist_get_nvlist(nvl, name));
 		else if (nvlist_exists(nvl, name)) {
 			for (copy = tofree; copy < name; copy++)
 				if (*copy == '\0')
@@ -76,6 +86,10 @@ _lookup_config_node(nvlist_t *parent, const char *path, bool create)
 			nvl = NULL;
 			break;
 		} else if (create) {
+			/*
+			 * XXX-MJ as with the case above, "new_nvl" shouldn't be
+			 * mutated after its ownership is given to "nvl".
+			 */
 			new_nvl = nvlist_create(0);
 			if (new_nvl == NULL)
 				errx(4, "Failed to allocate memory");