thr3ads.net - Libguestfs - [Libguestfs] [nbdkit PATCH] plugin: add and use nbdkit

If this information is useful, please help other people find it:
Share via:

Pino Toscano

2018-Feb-13 15:44 UTC

[Libguestfs] [nbdkit PATCH] plugin: add and use nbdkit_realpath

Introduce a new helper function to resolve a path name, calling
nbdkit_error on failure: other than doing what nbdkit_absolute_path
does, it also checks that the file exist (and thus avoid errors later
on).

Apply it where an existing path is required, both in nbdkit itself and
in plugins.

Related to: https://bugzilla.redhat.com/show_bug.cgi?id=1527334
---
 docs/nbdkit-plugin.pod      | 13 +++++++++++++
 include/nbdkit-common.h     |  1 +
 plugins/example2/example2.c |  2 +-
 plugins/file/file.c         |  6 +-----
 plugins/gzip/gzip.c         |  2 +-
 plugins/nbd/nbd.c           |  2 +-
 plugins/split/split.c       |  2 +-
 plugins/vddk/vddk.c         |  2 +-
 plugins/xz/xz.c             |  2 +-
 src/plugins.c               |  2 +-
 src/utils.c                 | 19 +++++++++++++++++++
 11 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index 44822fc..4576635 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -207,6 +207,19 @@ C<NULL>.  Note that this function does not check that
the file exists.
 
 The returned string must be freed by the caller.
 
+=head2 C<nbdkit_realpath>
+
+ char *nbdkit_realpath (const char *filename);
+
+The utility function C<nbdkit_realpath> converts any path to an
+absolute path, resolving symlinks.  Under the hood, it uses the
+C<realpath> function.
+
+If the path resolution was not possible, this calls C<nbdkit_error>
+and returns C<NULL>.
+
+The returned string must be freed by the caller.
+
 =head1 CALLBACKS
 
 =head2 C<.name>
diff --git a/include/nbdkit-common.h b/include/nbdkit-common.h
index 5e69579..693213f 100644
--- a/include/nbdkit-common.h
+++ b/include/nbdkit-common.h
@@ -60,6 +60,7 @@ extern void nbdkit_vdebug (const char *msg, va_list args);
 extern char *nbdkit_absolute_path (const char *path);
 extern int64_t nbdkit_parse_size (const char *str);
 extern int nbdkit_read_password (const char *value, char **password);
+extern char *nbdkit_realpath (const char *path);
 
 #ifdef __cplusplus
 }
diff --git a/plugins/example2/example2.c b/plugins/example2/example2.c
index 5bc4f94..a2d6fca 100644
--- a/plugins/example2/example2.c
+++ b/plugins/example2/example2.c
@@ -78,7 +78,7 @@ example2_config (const char *key, const char *value)
 {
   if (strcmp (key, "file") == 0) {
     /* See FILENAMES AND PATHS in nbdkit-plugin(3). */
-    filename = nbdkit_absolute_path (value);
+    filename = nbdkit_realpath (value);
     if (!filename)
       return -1;
   }
diff --git a/plugins/file/file.c b/plugins/file/file.c
index f8cb3d3..b6e33de 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -65,7 +65,7 @@ file_config (const char *key, const char *value)
   if (strcmp (key, "file") == 0) {
     /* See FILENAMES AND PATHS in nbdkit-plugin(3). */
     free (filename);
-    filename = nbdkit_absolute_path (value);
+    filename = nbdkit_realpath (value);
     if (!filename)
       return -1;
   }
@@ -90,10 +90,6 @@ file_config_complete (void)
     nbdkit_error ("you must supply the file=<FILENAME> parameter
after the plugin name on the command line");
     return -1;
   }
-  if (access (filename, F_OK) < 0) {
-    nbdkit_error ("access '%s': %m", filename);
-    return -1;
-  }
 
   return 0;
 }
diff --git a/plugins/gzip/gzip.c b/plugins/gzip/gzip.c
index e9dbfdb..09dd629 100644
--- a/plugins/gzip/gzip.c
+++ b/plugins/gzip/gzip.c
@@ -62,7 +62,7 @@ gzip_config (const char *key, const char *value)
 {
   if (strcmp (key, "file") == 0) {
     /* See FILENAMES AND PATHS in nbdkit-plugin(3). */
-    filename = nbdkit_absolute_path (value);
+    filename = nbdkit_realpath (value);
     if (!filename)
       return -1;
   }
diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
index c9727f7..9d0854c 100644
--- a/plugins/nbd/nbd.c
+++ b/plugins/nbd/nbd.c
@@ -68,7 +68,7 @@ nbd_config (const char *key, const char *value)
   if (strcmp (key, "socket") == 0) {
     /* See FILENAMES AND PATHS in nbdkit-plugin(3) */
     free (sockname);
-    sockname = nbdkit_absolute_path (value);
+    sockname = nbdkit_realpath (value);
     if (!sockname)
       return -1;
   }
diff --git a/plugins/split/split.c b/plugins/split/split.c
index 47c366d..bdcdcf7 100644
--- a/plugins/split/split.c
+++ b/plugins/split/split.c
@@ -76,7 +76,7 @@ split_config (const char *key, const char *value)
       return -1;
     }
     filenames = new_filenames;
-    filenames[nr_files] = nbdkit_absolute_path (value);
+    filenames[nr_files] = nbdkit_realpath (value);
     if (filenames[nr_files] == NULL)
       return -1;
     nr_files++;
diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c
index 1c15127..8bc1517 100644
--- a/plugins/vddk/vddk.c
+++ b/plugins/vddk/vddk.c
@@ -153,7 +153,7 @@ vddk_config (const char *key, const char *value)
   if (strcmp (key, "config") == 0) {
     /* See FILENAMES AND PATHS in nbdkit-plugin(3). */
     free (config);
-    config = nbdkit_absolute_path (value);
+    config = nbdkit_realpath (value);
     if (!config)
       return -1;
   }
diff --git a/plugins/xz/xz.c b/plugins/xz/xz.c
index 437f798..f45e489 100644
--- a/plugins/xz/xz.c
+++ b/plugins/xz/xz.c
@@ -67,7 +67,7 @@ xz_config (const char *key, const char *value)
 {
   if (strcmp (key, "file") == 0) {
     /* See FILENAMES AND PATHS in nbdkit-plugin(3). */
-    filename = nbdkit_absolute_path (value);
+    filename = nbdkit_realpath (value);
     if (!filename)
       return -1;
   }
diff --git a/src/plugins.c b/src/plugins.c
index dba3e24..595b632 100644
--- a/src/plugins.c
+++ b/src/plugins.c
@@ -134,7 +134,7 @@ plugin_dump_fields (struct backend *b)
   struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
   char *path;
 
-  path = nbdkit_absolute_path (p->filename);
+  path = nbdkit_realpath (p->filename);
   printf ("path=%s\n", path);
   free (path);
 
diff --git a/src/utils.c b/src/utils.c
index 0083370..671b662 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -228,3 +228,22 @@ nbdkit_read_password (const char *value, char **password)
 
   return 0;
 }
+
+char *
+nbdkit_realpath (const char *path)
+{
+  char *ret;
+
+  if (path == NULL || *path == '\0') {
+    nbdkit_error ("cannot resolve a null or empty path");
+    return NULL;
+  }
+
+  ret = realpath (path, NULL);
+  if (ret == NULL) {
+    nbdkit_error ("realpath: %m");
+    return NULL;
+  }
+
+  return ret;
+}
-- 
2.14.3

Eric Blake

2018-Feb-14 14:29 UTC

head link

Re: [Libguestfs] [nbdkit PATCH] plugin: add and use nbdkit_realpath

On 02/13/2018 09:44 AM, Pino Toscano wrote:> Introduce a new helper function to resolve a path name, calling
> nbdkit_error on failure: other than doing what nbdkit_absolute_path
> does, it also checks that the file exist (and thus avoid errors later
s/exist/exists/
s/avoid/avoids/
> on).
> 
> Apply it where an existing path is required, both in nbdkit itself and
> in plugins.
> 
> Related to: https://bugzilla.redhat.com/show_bug.cgi?id=1527334
> ---
>   docs/nbdkit-plugin.pod      | 13 +++++++++++++
>   include/nbdkit-common.h     |  1 +
Should we also document the helper functions in docs/nbdkit-filter.pod?

Otherwise looks good to me

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Pino Toscano

2018-Feb-14 15:37 UTC

head link

Re: [Libguestfs] [nbdkit PATCH] plugin: add and use nbdkit_realpath

On Wednesday, 14 February 2018 15:29:39 CET Eric Blake
wrote:> On 02/13/2018 09:44 AM, Pino Toscano wrote:
> > Introduce a new helper function to resolve a path name, calling
> > nbdkit_error on failure: other than doing what nbdkit_absolute_path
> > does, it also checks that the file exist (and thus avoid errors later
> 
> s/exist/exists/
> s/avoid/avoids/
Fixed locally, thanks.
> > on).
> > 
> > Apply it where an existing path is required, both in nbdkit itself and
> > in plugins.
> > 
> > Related to: https://bugzilla.redhat.com/show_bug.cgi?id=1527334
> > ---
> >   docs/nbdkit-plugin.pod      | 13 +++++++++++++
> >   include/nbdkit-common.h     |  1 +
> 
> Should we also document the helper functions in docs/nbdkit-filter.pod?
If so, that will be a different patch though, since nbdkit-filter does
not document the plugin API, but links to it.

-- 
Pino Toscano

Richard W.M. Jones

2018-Feb-14 16:03 UTC

head link

Re: [Libguestfs] [nbdkit PATCH] plugin: add and use nbdkit_realpath

On Tue, Feb 13, 2018 at 04:44:02PM +0100, Pino Toscano
wrote:> Introduce a new helper function to resolve a path name, calling
> nbdkit_error on failure: other than doing what nbdkit_absolute_path
> does, it also checks that the file exist (and thus avoid errors later
> on).
> 
> Apply it where an existing path is required, both in nbdkit itself and
> in plugins.
> 
> Related to: https://bugzilla.redhat.com/show_bug.cgi?id=1527334
> ---
>  docs/nbdkit-plugin.pod      | 13 +++++++++++++
>  include/nbdkit-common.h     |  1 +
>  plugins/example2/example2.c |  2 +-
>  plugins/file/file.c         |  6 +-----
>  plugins/gzip/gzip.c         |  2 +-
>  plugins/nbd/nbd.c           |  2 +-
>  plugins/split/split.c       |  2 +-
>  plugins/vddk/vddk.c         |  2 +-
>  plugins/xz/xz.c             |  2 +-
>  src/plugins.c               |  2 +-
>  src/utils.c                 | 19 +++++++++++++++++++
>  11 files changed, 41 insertions(+), 12 deletions(-)
> 
> diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
> index 44822fc..4576635 100644
> --- a/docs/nbdkit-plugin.pod
> +++ b/docs/nbdkit-plugin.pod
> @@ -207,6 +207,19 @@ C<NULL>.  Note that this function does not check
that the file exists.
>  
>  The returned string must be freed by the caller.
>  
> +=head2 C<nbdkit_realpath>
> +
> + char *nbdkit_realpath (const char *filename);
> +
> +The utility function C<nbdkit_realpath> converts any path to an
> +absolute path, resolving symlinks.  Under the hood, it uses the
> +C<realpath> function.
Can it say something about the difference between this one and
nbdkit_absolute_path?  Like, that the file / directory must exist
otherwise it's an error.  I think otherwise people would wonder why
there are two functions that seem to do the same thing.

Rest of the patch seems fine, so ACK with updated documentation.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Possibly Parallel Threads

Search for more apparently analagous threads

Libguestfs - Feb 2018 - [nbdkit PATCH] plugin: add and use nbdkit_realpath

[Libguestfs] [nbdkit PATCH] plugin: add and use nbdkit_realpath

Re: [Libguestfs] [nbdkit PATCH] plugin: add and use nbdkit_realpath

Re: [Libguestfs] [nbdkit PATCH] plugin: add and use nbdkit_realpath

Re: [Libguestfs] [nbdkit PATCH] plugin: add and use nbdkit_realpath

Possibly Parallel Threads