Re: [PATCH v2 12/13] block/ssh: Use URI parsing code from glib
Eric Blake writes: > On Fri, Apr 12, 2024 at 03:24:14PM +0200, Thomas Huth wrote: >> Since version 2.66, glib has useful URI parsing functions, too. >> Use those instead of the QEMU-internal ones to be finally able >> to get rid of the latter. >> >> Reviewed-by: Richard W.M. Jones >> Signed-off-by: Thomas Huth >> --- >> block/ssh.c | 75 - >> 1 file changed, 40 insertions(+), 35 deletions(-) >> > >> >> -if (g_strcmp0(uri->scheme, "ssh") != 0) { >> +if (g_strcmp0(g_uri_get_scheme(uri), "ssh") != 0) { > > Yet another case-sensitive spot to consider. > >> >> -qdict_put_str(options, "path", uri->path); >> - >> -/* Pick out any query parameters that we understand, and ignore >> - * the rest. >> - */ >> -for (i = 0; i < qp->n; ++i) { >> -if (strcmp(qp->p[i].name, "host_key_check") == 0) { >> -qdict_put_str(options, "host_key_check", qp->p[i].value); >> +qdict_put_str(options, "path", uri_path); >> + >> +uri_query = g_uri_get_query(uri); >> +if (uri_query) { >> +g_uri_params_iter_init(, uri_query, -1, "&", G_URI_PARAMS_NONE); >> +while (g_uri_params_iter_next(, _name, _value, )) { >> +if (!qp_name || !qp_value || gerror) { >> +warn_report("Failed to parse SSH URI parameters '%s'.", >> +uri_query); >> +break; >> +} >> +/* >> + * Pick out the query parameters that we understand, and ignore >> + * (or rather warn about) the rest. >> + */ >> +if (g_str_equal(qp_name, "host_key_check")) { >> +qdict_put_str(options, "host_key_check", qp_value); >> +} else { >> +warn_report("Unsupported parameter '%s' in URI.", qp_name); > > Do we want the trailing '.' in warn_report? We do not. > The warning is new; it was not in the old code, nor mentioned in the > commit message. It seems like a good idea, but we should be more > intentional if we intend to make that change.
Re: [PATCH v2 12/13] block/ssh: Use URI parsing code from glib
On Fri, Apr 12, 2024 at 03:24:14PM +0200, Thomas Huth wrote: > Since version 2.66, glib has useful URI parsing functions, too. > Use those instead of the QEMU-internal ones to be finally able > to get rid of the latter. > > Reviewed-by: Richard W.M. Jones > Signed-off-by: Thomas Huth > --- > block/ssh.c | 75 - > 1 file changed, 40 insertions(+), 35 deletions(-) > > > -if (g_strcmp0(uri->scheme, "ssh") != 0) { > +if (g_strcmp0(g_uri_get_scheme(uri), "ssh") != 0) { Yet another case-sensitive spot to consider. > > -qdict_put_str(options, "path", uri->path); > - > -/* Pick out any query parameters that we understand, and ignore > - * the rest. > - */ > -for (i = 0; i < qp->n; ++i) { > -if (strcmp(qp->p[i].name, "host_key_check") == 0) { > -qdict_put_str(options, "host_key_check", qp->p[i].value); > +qdict_put_str(options, "path", uri_path); > + > +uri_query = g_uri_get_query(uri); > +if (uri_query) { > +g_uri_params_iter_init(, uri_query, -1, "&", G_URI_PARAMS_NONE); > +while (g_uri_params_iter_next(, _name, _value, )) { > +if (!qp_name || !qp_value || gerror) { > +warn_report("Failed to parse SSH URI parameters '%s'.", > +uri_query); > +break; > +} > +/* > + * Pick out the query parameters that we understand, and ignore > + * (or rather warn about) the rest. > + */ > +if (g_str_equal(qp_name, "host_key_check")) { > +qdict_put_str(options, "host_key_check", qp_value); > +} else { > +warn_report("Unsupported parameter '%s' in URI.", qp_name); Do we want the trailing '.' in warn_report? The warning is new; it was not in the old code, nor mentioned in the commit message. It seems like a good idea, but we should be more intentional if we intend to make that change. -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org