Richard W.M. Jones
2023-Sep-09 13:57 UTC
[Libguestfs] [PATCH nbdkit 04/10] server: Calculate $uri in one place
Move the calculation of $uri to the main function (well, inlined
there), and out of --run code.
This is largely code motion. In theory it changes the content of $uri
since we now shell quote it after generating it, but this ought not to
have any practical effect.
---
server/internal.h | 1 +
server/captive.c | 41 ++----------------------
server/main.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 82 insertions(+), 39 deletions(-)
diff --git a/server/internal.h b/server/internal.h
index 0652f8a86..656b6d8ce 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -142,6 +142,7 @@ extern const char *user, *group;
extern bool verbose;
extern bool vsock;
extern enum service_mode service_mode;
+extern char *uri;
extern bool configured;
extern int saved_stdin;
extern int saved_stdout;
diff --git a/server/captive.c b/server/captive.c
index 40c4bb2ca..51dafca34 100644
--- a/server/captive.c
+++ b/server/captive.c
@@ -75,45 +75,8 @@ run_command (void)
/* Construct $uri. */
fprintf (fp, "uri=");
- switch (service_mode) {
- case SERVICE_MODE_SOCKET_ACTIVATION:
- case SERVICE_MODE_LISTEN_STDIN:
- break; /* can't form a URI, leave it blank */
- 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);
- break;
- case SERVICE_MODE_VSOCK:
- /* 1 = VMADDR_CID_LOCAL */
- fprintf (fp, "nbd%s+vsock://1", tls == 2 ? "s" :
"");
- if (port) {
- putc (':', fp);
- shell_quote (port, fp);
- }
- if (export_name && strcmp (export_name, "") != 0) {
- putc ('/', fp);
- uri_quote (export_name, fp);
- }
- break;
- case SERVICE_MODE_TCPIP:
- fprintf (fp, "nbd%s://localhost", tls == 2 ? "s" :
"");
- if (port) {
- putc (':', fp);
- shell_quote (port, fp);
- }
- if (export_name && strcmp (export_name, "") != 0) {
- putc ('/', fp);
- uri_quote (export_name, fp);
- }
- break;
- default:
- abort ();
- }
+ if (uri)
+ shell_quote (uri, fp);
putc ('\n', fp);
/* Since nbdkit 1.24, $nbd is a synonym for $uri. */
diff --git a/server/main.c b/server/main.c
index 2a332bfdd..54eb348ba 100644
--- a/server/main.c
+++ b/server/main.c
@@ -66,6 +66,7 @@
#include "ascii-string.h"
#include "exit-with-parent.h"
#include "nbd-protocol.h"
+#include "open_memstream.h"
#include "realpath.h"
#include "strndup.h"
#include "syslog.h"
@@ -79,6 +80,7 @@
#endif
static char *make_random_fifo (void);
+static char *make_uri (void);
static struct backend *open_plugin_so (size_t i, const char *filename,
int short_name);
static struct backend *open_filter_so (struct backend *next, size_t i,
@@ -118,6 +120,7 @@ bool verbose; /* -v */
bool vsock; /* --vsock */
unsigned int socket_activation; /* $LISTEN_FDS and $LISTEN_PID set */
enum service_mode service_mode; /* serving over TCP, Unix, etc */
+char *uri; /* NBD URI */
bool configured; /* .config_complete done */
int saved_stdin = -1; /* dup'd stdin during -s/--run */
int saved_stdout = -1; /* dup'd stdout during -s/--run */
@@ -630,6 +633,11 @@ main (int argc, char *argv[])
else
service_mode = SERVICE_MODE_TCPIP;
+ /* By the point we have enough information to calculate the NBD URI.
+ * Note this may be NULL.
+ */
+ uri = make_uri ();
+
/* The remaining command line arguments are the plugin name and
* parameters. If --help, --version or --dump-plugin were specified
* then we open the plugin so that we can display the per-plugin
@@ -799,6 +807,7 @@ main (int argc, char *argv[])
free (unixsocket);
free (pidfile);
+ free (uri);
if (random_fifo) {
unlink (random_fifo);
@@ -856,6 +865,76 @@ make_random_fifo (void)
return NULL;
}
+static char *
+make_uri (void)
+{
+ FILE *fp;
+ size_t len = 0;
+ char *r = NULL;
+
+ switch (service_mode) {
+ case SERVICE_MODE_SOCKET_ACTIVATION:
+ case SERVICE_MODE_LISTEN_STDIN:
+ /* can't form a URI, uri will be NULL */
+ return NULL;
+ default: ;
+ }
+
+ fp = open_memstream (&r, &len);
+ if (fp == NULL) {
+ perror ("open_memstream");
+ exit (EXIT_FAILURE);
+ }
+
+ 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);
+ break;
+ case SERVICE_MODE_VSOCK:
+ /* 1 = VMADDR_CID_LOCAL */
+ fprintf (fp, "nbd%s+vsock://1", tls == 2 ? "s" :
"");
+ if (port) {
+ putc (':', fp);
+ fputs (port, fp);
+ }
+ if (export_name && strcmp (export_name, "") != 0) {
+ putc ('/', fp);
+ uri_quote (export_name, fp);
+ }
+ break;
+ case SERVICE_MODE_TCPIP:
+ fprintf (fp, "nbd%s://localhost", tls == 2 ? "s" :
"");
+ if (port) {
+ putc (':', fp);
+ fputs (port, fp);
+ }
+ if (export_name && strcmp (export_name, "") != 0) {
+ putc ('/', fp);
+ uri_quote (export_name, fp);
+ }
+ break;
+
+ case SERVICE_MODE_SOCKET_ACTIVATION:
+ case SERVICE_MODE_LISTEN_STDIN:
+ abort (); /* see above */
+ default:
+ abort ();
+ }
+
+ if (fclose (fp) == EOF) {
+ perror ("memstream failed");
+ exit (EXIT_FAILURE);
+ }
+
+ return r;
+}
+
static struct backend *
open_plugin_so (size_t i, const char *name, int short_name)
{
--
2.41.0
Eric Blake
2023-Sep-11 14:01 UTC
[Libguestfs] [PATCH nbdkit 04/10] server: Calculate $uri in one place
On Sat, Sep 09, 2023 at 02:57:52PM +0100, Richard W.M. Jones wrote:> Move the calculation of $uri to the main function (well, inlined > there), and out of --run code. > > This is largely code motion. In theory it changes the content of $uri > since we now shell quote it after generating it, but this ought not to > have any practical effect. > --- > server/internal.h | 1 + > server/captive.c | 41 ++---------------------- > server/main.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 82 insertions(+), 39 deletions(-) >> +++ b/server/captive.c > @@ -75,45 +75,8 @@ run_command (void) > > /* Construct $uri. */ > fprintf (fp, "uri="); > - switch (service_mode) { > - case SERVICE_MODE_SOCKET_ACTIVATION: > - case SERVICE_MODE_LISTEN_STDIN: > - break; /* can't form a URI, leave it blank */ > - 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...> - break; > - case SERVICE_MODE_VSOCK: > - /* 1 = VMADDR_CID_LOCAL */ > - fprintf (fp, "nbd%s+vsock://1", tls == 2 ? "s" : ""); > - if (port) { > - putc (':', fp); > - shell_quote (port, fp); > - } > - if (export_name && strcmp (export_name, "") != 0) { > - putc ('/', fp); > - uri_quote (export_name, fp); > - } > - break; > - case SERVICE_MODE_TCPIP: > - fprintf (fp, "nbd%s://localhost", tls == 2 ? "s" : ""); > - if (port) { > - putc (':', fp); > - shell_quote (port, fp); > - } > - if (export_name && strcmp (export_name, "") != 0) { > - putc ('/', fp); > - uri_quote (export_name, fp); > - } > - break; > - default: > - abort (); > - } > + if (uri) > + shell_quote (uri, fp);...while here, we shell-quote the entire string...> putc ('\n', fp); > > /* Since nbdkit 1.24, $nbd is a synonym for $uri. */ > diff --git a/server/main.c b/server/main.c > index 2a332bfdd..54eb348ba 100644 > --- a/server/main.c> +static char * > +make_uri (void)> + > + 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).> + break; > + case SERVICE_MODE_VSOCK: > + /* 1 = VMADDR_CID_LOCAL */ > + fprintf (fp, "nbd%s+vsock://1", tls == 2 ? "s" : ""); > + if (port) { > + putc (':', fp); > + fputs (port, fp); > + } > + if (export_name && strcmp (export_name, "") != 0) { > + putc ('/', fp); > + uri_quote (export_name, fp); > + } > + break; > + case SERVICE_MODE_TCPIP: > + fprintf (fp, "nbd%s://localhost", tls == 2 ? "s" : ""); > + if (port) { > + putc (':', fp); > + fputs (port, fp); > + } > + if (export_name && strcmp (export_name, "") != 0) { > + putc ('/', fp); > + uri_quote (export_name, fp); > + } > + break; > + > + case SERVICE_MODE_SOCKET_ACTIVATION: > + case SERVICE_MODE_LISTEN_STDIN: > + abort (); /* see above */ > + default: > + abort ();Could consolidate these labels, although I don't know if a compiler would be picky about: case ...: /* comment */ default: abort (); so I'm also fine leaving it as-is. Reviewed-by: Eric Blake <eblake at redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org