On Fri, Dec 10, 2021 at 05:47:41AM -0800, Andrea Bolognani wrote: > On Fri, Dec 10, 2021 at 11:31:19AM +0100, Peter Krempa wrote: > > On Fri, Dec 10, 2021 at 10:59:27 +0100, Andrea Bolognani wrote: > > > @@ -69,7 +69,15 @@ remoteSplitURIScheme(virURI *uri, > > > char **driver, > > > remoteDriverTransport *transport) > > > { > > > - char *p = strchr(uri->scheme, '+'); > > > + char *p = NULL; > > > + > > > + if (!uri->scheme) { > > > + virReportError(VIR_ERR_INVALID_ARG, "%s", > > > + _("missing scheme for URI")); > > > > The other place which leads to the call of this helper (virConnectOpenInternal) > > uses the following error to reject the uri if scheme is missing: > > > > virReportError(VIR_ERR_NO_CONNECT, > > _("URI '%s' does not include a driver name"), > > name); > > Yeah, it seems safer to catch the issue inside the helper than > requiring the callers to perform the check ahead of time. It's okay > for virConnectOpen() to have a nicer error message, as it's the one > that people are more likely to see. Yes, this is something I simply overlooked when refactoring the code. The check should clearly be in this common helper. > I entertained the thought of adding the check to virURIParse() > directly, because I can't think of a scenario where having a NULL > scheme would be considered valid. But that seemed like a change that > had the potential to break unrelated stuff, so I cowardly decided to > go with the safe version instead O:-) We've supported URIs without a scheme in the past. IIRC, we allowed a bath path to a UNIX socket for the original Xen driver. That code is deleted now of course. 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 :|