Date: Wed, 11 Aug 2021 20:06:39 +0200 From: Ronald Klop <ronald-lists@klop.ws> To: Alan Somers <asomers@FreeBSD.org>, src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: 6c9506559080 - main - Escape any '.' characters in sysctl node names Message-ID: <076f21f0-c6f5-1137-f1e6-b251e01b456a@klop.ws> In-Reply-To: <202107221623.16MGNZDu023290@gitrepo.freebsd.org> References: <202107221623.16MGNZDu023290@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 7/22/21 6:23 PM, Alan Somers wrote: > The branch main has been updated by asomers: > > URL: https://cgit.FreeBSD.org/src/commit/?id=6c9506559080da2914749bf611225d7c0a153609 > > commit 6c9506559080da2914749bf611225d7c0a153609 > Author: Alan Somers <asomers@FreeBSD.org> > AuthorDate: 2021-07-21 21:11:00 +0000 > Commit: Alan Somers <asomers@FreeBSD.org> > CommitDate: 2021-07-22 16:22:48 +0000 > > Escape any '.' characters in sysctl node names > > ZFS creates some sysctl nodes that include a pool name, and '.' is an > allowed character in pool names. But it's the separator in the sysctl > tree, so it can't be included in a sysctl name. Replace it with "%25". > Handily, "%" is illegal in ZFS pool names, so there's no ambiguity > there. Hi, Wouldn't it be cleaner to enumerate the pools as numbers/ids and put the name of the pool in a field as the data instead of the key? Regards, Ronald. > PR: 257316 > MFC after: 3 weeks > Sponsored by: Axcient > Reviewed by: freqlabs > Differential Revision: https://reviews.freebsd.org/D31265 > --- > sys/kern/kern_sysctl.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 46 insertions(+), 2 deletions(-) > > diff --git a/sys/kern/kern_sysctl.c b/sys/kern/kern_sysctl.c > index e46584758c9b..c472db18aac7 100644 > --- a/sys/kern/kern_sysctl.c > +++ b/sys/kern/kern_sysctl.c > @@ -122,6 +122,7 @@ static int sysctl_root(SYSCTL_HANDLER_ARGS); > /* Root list */ > struct sysctl_oid_list sysctl__children = SLIST_HEAD_INITIALIZER(&sysctl__children); > > +static char* sysctl_escape_name(const char*); > static int sysctl_remove_oid_locked(struct sysctl_oid *oidp, int del, > int recurse); > static int sysctl_old_kernel(struct sysctl_req *, const void *, size_t); > @@ -747,6 +748,46 @@ sysctl_remove_name(struct sysctl_oid *parent, const char *name, > return (error); > } > > +/* > + * Duplicate the provided string, escaping any illegal characters. The result > + * must be freed when no longer in use. > + * > + * The list of illegal characters is ".". > + */ > +static char* > +sysctl_escape_name(const char* orig) > +{ > + int i, s = 0, d = 0, nillegals = 0; > + char *new; > + > + /* First count the number of illegal characters */ > + for (i = 0; orig[i] != '\0'; i++) { > + if (orig[i] == '.') > + nillegals++; > + } > + > + /* Allocate storage for new string */ > + new = malloc(i + 2 * nillegals + 1, M_SYSCTLOID, M_WAITOK); > + > + /* Copy the name, escaping characters as we go */ > + while (orig[s] != '\0') { > + if (orig[s] == '.') { > + /* %25 is the hexadecimal representation of '.' */ > + new[d++] = '%'; > + new[d++] = '2'; > + new[d++] = '5'; > + s++; > + } else { > + new[d++] = orig[s++]; > + } > + } > + > + /* Finally, nul-terminate */ > + new[d] = '\0'; > + > + return (new); > +} > + > static int > sysctl_remove_oid_locked(struct sysctl_oid *oidp, int del, int recurse) > { > @@ -828,14 +869,17 @@ sysctl_add_oid(struct sysctl_ctx_list *clist, struct sysctl_oid_list *parent, > const char *label) > { > struct sysctl_oid *oidp; > + char *escaped; > > /* You have to hook up somewhere.. */ > if (parent == NULL) > return(NULL); > + escaped = sysctl_escape_name(name); > /* Check if the node already exists, otherwise create it */ > SYSCTL_WLOCK(); > - oidp = sysctl_find_oidname(name, parent); > + oidp = sysctl_find_oidname(escaped, parent); > if (oidp != NULL) { > + free(escaped, M_SYSCTLOID); > if ((oidp->oid_kind & CTLTYPE) == CTLTYPE_NODE) { > oidp->oid_refcnt++; > /* Update the context */ > @@ -854,7 +898,7 @@ sysctl_add_oid(struct sysctl_ctx_list *clist, struct sysctl_oid_list *parent, > SLIST_INIT(&oidp->oid_children); > oidp->oid_number = number; > oidp->oid_refcnt = 1; > - oidp->oid_name = strdup(name, M_SYSCTLOID); > + oidp->oid_name = escaped; > oidp->oid_handler = handler; > oidp->oid_kind = CTLFLAG_DYN | kind; > oidp->oid_arg1 = arg1; > _______________________________________________ > dev-commits-src-all@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/dev-commits-src-all > To unsubscribe, send any mail to "dev-commits-src-all-unsubscribe@freebsd.org" >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?076f21f0-c6f5-1137-f1e6-b251e01b456a>