Re: [Libguestfs] [PATCH nbdkit 04/10] server: Calculate $uri in one place

On Mon, Sep 11, 2023 at 03:09:21PM +0100, Richard W.M. Jones wrote: > > > - case SERVICE_MODE_UNIXSOCKET: > > > - fprintf (fp, "nbd%s+unix://", tls == 2 ? "s" : ""); > > > - if (export_name && strcmp (export_name, "") != 0) { > > > - putc ('/', fp); > > > - uri_quote (export_name, fp); > > > - } > > > - fprintf (fp, "\\?socket="); > > > - uri_quote (unixsocket, fp); > > > > Beforehand, we were manually shell-quoting the ? in the Unix URI... > > The shell quoting here was only marginally useful before this change. > In theory there might be a file in a subdirectory called > 'nbd+unix:/Xsocket=' which would match :-) > > > + switch (service_mode) { > > > + case SERVICE_MODE_UNIXSOCKET: > > > + fprintf (fp, "nbd%s+unix://", tls == 2 ? "s" : ""); > > > + if (export_name && strcmp (export_name, "") != 0) { > > > + putc ('/', fp); > > > + uri_quote (export_name, fp); > > > + } > > > + fprintf (fp, "?socket="); > > > + uri_quote (unixsocket, fp); > > > > ...where the manual shell-quoting is no longer injected. Yes, this > > looks correct (the appearance of the quoting, using '' instead of \, > > may be different, but the resulting string as parsed by the shell is > > the same). My point was that pre-patch, we had: nbd+unix://\?socket=... and post-patch, we have: 'nbd+unix://?socket=...' but both forms are equally quoted; after the shell removes \ or '' quoting, we are left with: nbd+unix://?socket=... and neither form allowed the glob expansion of the (unlikely) file nbd+unix:/Xsocket=... My comment was more about why changing the fprintf("\\?socket=") pre-patch to fprintf("?socket=") post-patch was correct.