Libguestfs - Aug 2020 - [nbdkit PATCH v2] server: Permit - in plugin names

Use of - does not need shell quoting, and aids legibility in
multi-word plugin or filter names.  Permitting both - and _ would be
ambiguous (not to mention that things like 'man nbdkit-foo_bar-plugin'
would look ugly), so prefer only the character that is easier for
human use.  Permitting a leading - would be ambiguous with options,
but restricting to a letter as the first character would rule out any
existing plugins such as a theoretical '9p' plugin for interaction
with the 9p filesystem.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

After a bit more IRC chat, I think this is more what we want.

 docs/nbdkit-filter.pod |  3 ++-
 docs/nbdkit-plugin.pod |  3 ++-
 server/backend.c       | 15 +++++++++++----
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod
index 12343dbf..93601088 100644
--- a/docs/nbdkit-filter.pod
+++ b/docs/nbdkit-filter.pod
@@ -201,7 +201,8 @@ methods.
  const char *name;

 This field (a string) is required, and B<must> contain only ASCII
-alphanumeric characters and be unique amongst all filters.
+alphanumeric characters or non-leading dashes, and be unique amongst
+all filters.

 =head2 C<.longname>

diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index 9341f282..a661680c 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -471,7 +471,8 @@ an indication of failure.  It has the following prototype:
  const char *name;

 This field (a string) is required, and B<must> contain only ASCII
-alphanumeric characters and be unique amongst all plugins.
+alphanumeric characters or non-leading dashes, and be unique amongst
+all filters.

 =head2 C<.version>

diff --git a/server/backend.c b/server/backend.c
index 75ca53be..b001a9a9 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -98,13 +98,20 @@ backend_load (struct backend *b, const char *name, void
(*load) (void))
              program_name, b->filename, b->type);
     exit (EXIT_FAILURE);
   }
-  for (i = 0; i < len; ++i) {
+  if (! ascii_isalnum (*name)) {
+    fprintf (stderr,
+             "%s: %s: %s.name ('%s') field must begin with an
"
+             "ASCII alphanumeric\n",
+             program_name, b->filename, b->type, name);
+    exit (EXIT_FAILURE);
+  }
+  for (i = 1; i < len; ++i) {
     unsigned char c = name[i];

-    if (! ascii_isalnum (c)) {
+    if (! ascii_isalnum (c) && c != '-') {
       fprintf (stderr,
-               "%s: %s: %s.name ('%s') field "
-               "must contain only ASCII alphanumeric characters\n",
+               "%s: %s: %s.name ('%s') field must contain only
"
+               "ASCII alphanumeric or dash characters\n",
                program_name, b->filename, b->type, name);
       exit (EXIT_FAILURE);
     }
-- 
2.28.0

On Mon, Aug 10, 2020 at 11:25:31AM -0500, Eric Blake
wrote:
> Use of - does not need shell quoting, and aids legibility in
> multi-word plugin or filter names.  Permitting both - and _ would be
> ambiguous (not to mention that things like 'man
nbdkit-foo_bar-plugin'
> would look ugly), so prefer only the character that is easier for
> human use.  Permitting a leading - would be ambiguous with options,
> but restricting to a letter as the first character would rule out any
> existing plugins such as a theoretical '9p' plugin for interaction
> with the 9p filesystem.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> 
> After a bit more IRC chat, I think this is more what we want.
> 
>  docs/nbdkit-filter.pod |  3 ++-
>  docs/nbdkit-plugin.pod |  3 ++-
>  server/backend.c       | 15 +++++++++++----
>  3 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod
> index 12343dbf..93601088 100644
> --- a/docs/nbdkit-filter.pod
> +++ b/docs/nbdkit-filter.pod
> @@ -201,7 +201,8 @@ methods.
>   const char *name;
> 
>  This field (a string) is required, and B<must> contain only ASCII
> -alphanumeric characters and be unique amongst all filters.
> +alphanumeric characters or non-leading dashes, and be unique amongst
> +all filters.
> 
>  =head2 C<.longname>
> 
> diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
> index 9341f282..a661680c 100644
> --- a/docs/nbdkit-plugin.pod
> +++ b/docs/nbdkit-plugin.pod
> @@ -471,7 +471,8 @@ an indication of failure.  It has the following
prototype:
>   const char *name;
> 
>  This field (a string) is required, and B<must> contain only ASCII
> -alphanumeric characters and be unique amongst all plugins.
> +alphanumeric characters or non-leading dashes, and be unique amongst
> +all filters.
> 
>  =head2 C<.version>
> 
> diff --git a/server/backend.c b/server/backend.c
> index 75ca53be..b001a9a9 100644
> --- a/server/backend.c
> +++ b/server/backend.c
> @@ -98,13 +98,20 @@ backend_load (struct backend *b, const char *name, void
(*load) (void))
>               program_name, b->filename, b->type);
>      exit (EXIT_FAILURE);
>    }
> -  for (i = 0; i < len; ++i) {
> +  if (! ascii_isalnum (*name)) {
> +    fprintf (stderr,
> +             "%s: %s: %s.name ('%s') field must begin with an
"
> +             "ASCII alphanumeric\n",
> +             program_name, b->filename, b->type, name);
> +    exit (EXIT_FAILURE);
> +  }
> +  for (i = 1; i < len; ++i) {
>      unsigned char c = name[i];
> 
> -    if (! ascii_isalnum (c)) {
> +    if (! ascii_isalnum (c) && c != '-') {
>        fprintf (stderr,
> -               "%s: %s: %s.name ('%s') field "
> -               "must contain only ASCII alphanumeric
characters\n",
> +               "%s: %s: %s.name ('%s') field must contain
only "
> +               "ASCII alphanumeric or dash characters\n",
>                 program_name, b->filename, b->type, name);
>        exit (EXIT_FAILURE);
>      }
ACK

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html