Libguestfs - Apr 2018 - [nbdkit PATCH v3 1/2] docs: improve documentation of nbdkit_absolute_path

Hi,

this is the v3 of my series for nbdkit_realpath; this series adds an
extra documentation update to nbdkit_absolute_path, documenting when it
can only be used, and then adds nbdkit_realpath.

Thanks,

Pino Toscano (2):
  docs: improve documentation of nbdkit_absolute_path
  plugin: add and use nbdkit_realpath

 docs/nbdkit-plugin.pod      | 24 +++++++++++++++++++++++-
 include/nbdkit-common.h     |  1 +
 plugins/example2/example2.c |  2 +-
 plugins/file/file.c         |  6 +-----
 plugins/gzip/gzip.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 +++++++++++++++++++
 10 files changed, 50 insertions(+), 12 deletions(-)

-- 
2.14.3

Mention what it really does, and where it can only be used.

Signed-off-by: Pino Toscano <ptoscano@redhat.com>
---
 docs/nbdkit-plugin.pod | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index 65f3190..27a885e 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -273,7 +273,12 @@ descriptor.
  char *nbdkit_absolute_path (const char *filename);
 
 The utility function C<nbdkit_absolute_path> converts any path to an
-absolute path.
+absolute path: if it is relative, then all this function does is
+prepending the current working directory to the path, with no extra
+checks.
+
+Note that this function works I<only> when used in the C<.config>,
and
+C<.config_complete> callbacks.
 
 If conversion was not possible, this calls C<nbdkit_error> and returns
 C<NULL>.  Note that this function does not check that the file exists.
-- 
2.14.3

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 exists (and thus avoids 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

Signed-off-by: Pino Toscano <ptoscano@redhat.com>
---
 docs/nbdkit-plugin.pod      | 17 +++++++++++++++++
 include/nbdkit-common.h     |  1 +
 plugins/example2/example2.c |  2 +-
 plugins/file/file.c         |  6 +-----
 plugins/gzip/gzip.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 +++++++++++++++++++
 10 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index 27a885e..0fb81b8 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -285,6 +285,23 @@ 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, and thus it fails if the path does not exist,
+or it is not possible to access to any of the components of the path.
+
+Note that this function works I<only> when used in the C<.config>,
and
+C<.config_complete> callbacks.
+
+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 851b890..47d2c47 100644
--- a/include/nbdkit-common.h
+++ b/include/nbdkit-common.h
@@ -67,6 +67,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/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 4ff4b20..23223b3 100644
--- a/src/plugins.c
+++ b/src/plugins.c
@@ -133,7 +133,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 63adbf1..9134579 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -227,3 +227,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(%s): %m", path);
+    return NULL;
+  }
+
+  return ret;
+}
-- 
2.14.3

On 04/12/2018 07:44 AM, Pino Toscano wrote:
> Mention what it really does, and where it can only be used.
> 
> Signed-off-by: Pino Toscano <ptoscano@redhat.com>
> ---
>  docs/nbdkit-plugin.pod | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
> index 65f3190..27a885e 100644
> --- a/docs/nbdkit-plugin.pod
> +++ b/docs/nbdkit-plugin.pod
> @@ -273,7 +273,12 @@ descriptor.
>   char *nbdkit_absolute_path (const char *filename);
>  
>  The utility function C<nbdkit_absolute_path> converts any path to an
> -absolute path.
> +absolute path: if it is relative, then all this function does is
> +prepending the current working directory to the path, with no extra
s/prepending/prepend/
> +checks.
> +
> +Note that this function works I<only> when used in the
C<.config>, and
> +C<.config_complete> callbacks.
>  
>  If conversion was not possible, this calls C<nbdkit_error> and
returns
>  C<NULL>.  Note that this function does not check that the file
exists.
> 
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On 04/12/2018 07:44 AM, Pino Toscano wrote:
> Hi,
> 
> this is the v3 of my series for nbdkit_realpath; this series adds an
> extra documentation update to nbdkit_absolute_path, documenting when it
> can only be used, and then adds nbdkit_realpath.
I pointed out a typo fix in 1/2, but with that changed, it looks good to me.

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