Clarify the error message when unavailable functions are called (RHBZ#679737).

Callers are supposed to use the availability API to check for
functions that may not be available in particular builds of
libguestfs.  If they don't do this, currently they tend to get obscure
error messages, eg:

  libguestfs: error: zerofree: /dev/vda1: zerofree: No such file or directory

This commit changes the error message to explain what callers ought to
be doing instead:

  libguestfs: error: zerofree: feature 'zerofree' is not available in this
  build of libguestfs.  Read 'AVAILABILITY' in the guestfs(3) man page for
  how to check for the availability of features.

This patch makes the stubs check for availability.  The stub code
changes to:

  static void
  zerofree_stub (XDR *xdr_in)
  {
  [...]

    /* The caller should have checked before calling this. */
    if (! optgroup_zerofree_available ()) {
      reply_with_error ("feature '%s' is not available in this\n"
                        "build of libguestfs.  Read 'AVAILABILITY' in the guestfs(3) man page for\n"
                        "how to check for the availability of features.",
                        "zerofree");
      goto done;
    }
  [...]

daemon/augeas.c

@@ -63,19 +63,11 @@ optgroup_augeas_available (void)
 {
   return 1;
 }
-#else /* !HAVE_AUGEAS */
-int
-optgroup_augeas_available (void)
-{
-  return 0;
-}
-#endif
 
 /* We need to rewrite the root path so it is based at /sysroot. */
 int
 do_aug_init (const char *root, int flags)
 {
-#ifdef HAVE_AUGEAS
   char *buf;
 
   if (aug) {
@@ -98,24 +90,17 @@ do_aug_init (const char *root, int flags)
   }
 
   return 0;
-#else
-  NOT_AVAILABLE (-1);
-#endif
 }
 
 int
 do_aug_close (void)
 {
-#ifdef HAVE_AUGEAS
   NEED_AUG(-1);
 
   aug_close (aug);
   aug = NULL;
 
   return 0;
-#else
-  NOT_AVAILABLE (-1);
-#endif
 }
 
 int
@@ -133,7 +118,8 @@ do_aug_defvar (const char *name, const char *expr)
   }
   return r;
 #else
-  NOT_AVAILABLE (-1);
+  reply_with_error ("function not available");
+  return -1;
 #endif
 }
 
@@ -163,14 +149,14 @@ do_aug_defnode (const char *name, const char *expr, const char *val)
 
   return r;
 #else
-  NOT_AVAILABLE (NULL);
+  reply_with_error ("function not available");
+  return NULL;
 #endif
 }
 
 char *
 do_aug_get (const char *path)
 {
-#ifdef HAVE_AUGEAS
   const char *value = NULL;
   char *v;
   int r;
@@ -204,15 +190,11 @@ do_aug_get (const char *path)
   }
 
   return v;			/* Caller frees. */
-#else
-  NOT_AVAILABLE (NULL);
-#endif
 }
 
 int
 do_aug_set (const char *path, const char *val)
 {
-#ifdef HAVE_AUGEAS
   int r;
 
   NEED_AUG (-1);
@@ -224,15 +206,11 @@ do_aug_set (const char *path, const char *val)
   }
 
   return 0;
-#else
-  NOT_AVAILABLE (-1);
-#endif
 }
 
 int
 do_aug_clear (const char *path)
 {
-#ifdef HAVE_AUGEAS
   int r;
 
   NEED_AUG (-1);
@@ -244,15 +222,11 @@ do_aug_clear (const char *path)
   }
 
   return 0;
-#else
-  NOT_AVAILABLE (-1);
-#endif
 }
 
 int
 do_aug_insert (const char *path, const char *label, int before)
 {
-#ifdef HAVE_AUGEAS
   int r;
 
   NEED_AUG (-1);
@@ -264,15 +238,11 @@ do_aug_insert (const char *path, const char *label, int before)
   }
 
   return 0;
-#else
-  NOT_AVAILABLE (-1);
-#endif
 }
 
 int
 do_aug_rm (const char *path)
 {
-#ifdef HAVE_AUGEAS
   int r;
 
   NEED_AUG (-1);
@@ -284,15 +254,11 @@ do_aug_rm (const char *path)
   }
 
   return r;
-#else
-  NOT_AVAILABLE (-1);
-#endif
 }
 
 int
 do_aug_mv (const char *src, const char *dest)
 {
-#ifdef HAVE_AUGEAS
   int r;
 
   NEED_AUG (-1);
@@ -304,15 +270,11 @@ do_aug_mv (const char *src, const char *dest)
   }
 
   return 0;
-#else
-  NOT_AVAILABLE (-1);
-#endif
 }
 
 char **
 do_aug_match (const char *path)
 {
-#ifdef HAVE_AUGEAS
   char **matches = NULL;
   void *vp;
   int r;
@@ -338,15 +300,11 @@ do_aug_match (const char *path)
   matches[r] = NULL;
 
   return matches;		/* Caller frees. */
-#else
-  NOT_AVAILABLE (NULL);
-#endif
 }
 
 int
 do_aug_save (void)
 {
-#ifdef HAVE_AUGEAS
   NEED_AUG (-1);
 
   if (aug_save (aug) == -1) {
@@ -355,9 +313,6 @@ do_aug_save (void)
   }
 
   return 0;
-#else
-  NOT_AVAILABLE (-1);
-#endif
 }
 
 int
@@ -373,7 +328,8 @@ do_aug_load (void)
 
   return 0;
 #else
-  NOT_AVAILABLE (-1);
+  reply_with_error ("function not available");
+  return -1;
 #endif
 }
 
@@ -381,7 +337,6 @@ do_aug_load (void)
 char **
 do_aug_ls (const char *path)
 {
-#ifdef HAVE_AUGEAS
   char **matches;
   char *buf;
   int len;
@@ -420,7 +375,102 @@ do_aug_ls (const char *path)
 
   sort_strings (matches, count_strings ((void *) matches));
   return matches;		/* Caller frees. */
-#else
-  NOT_AVAILABLE (NULL);
-#endif
 }
+
+#else /* !HAVE_AUGEAS */
+
+/* Note that the wrapper code (daemon/stubs.c) ensures that the
+ * functions below are never called because optgroup_augeas_available
+ * returns false.
+ */
+int
+optgroup_augeas_available (void)
+{
+  return 0;
+}
+
+int
+do_aug_init (const char *root, int flags)
+{
+  abort ();
+}
+
+int
+do_aug_close (void)
+{
+  abort ();
+}
+
+int
+do_aug_defvar (const char *name, const char *expr)
+{
+  abort ();
+}
+
+guestfs_int_int_bool *
+do_aug_defnode (const char *name, const char *expr, const char *val)
+{
+  abort ();
+}
+
+char *
+do_aug_get (const char *path)
+{
+  abort ();
+}
+
+int
+do_aug_set (const char *path, const char *val)
+{
+  abort ();
+}
+
+int
+do_aug_clear (const char *path)
+{
+  abort ();
+}
+
+int
+do_aug_insert (const char *path, const char *label, int before)
+{
+  abort ();
+}
+
+int
+do_aug_rm (const char *path)
+{
+  abort ();
+}
+
+int
+do_aug_mv (const char *src, const char *dest)
+{
+  abort ();
+}
+
+char **
+do_aug_match (const char *path)
+{
+  abort ();
+}
+
+int
+do_aug_save (void)
+{
+  abort ();
+}
+
+int
+do_aug_load (void)
+{
+  abort ();
+}
+
+char **
+do_aug_ls (const char *path)
+{
+  abort ();
+}
+
+#endif

daemon/daemon.h

@@ -331,16 +331,6 @@ is_zero (const char *buffer, size_t size)
   }									\
   while (0)
 
-/* Marks functions which are not available.
- * NB. Cannot be used for FileIn functions.
- */
-#define NOT_AVAILABLE(errcode)                                          \
-  do {									\
-    reply_with_error ("%s: function not available", __func__);          \
-    return (errcode);							\
-  }									\
-  while (0)
-
 #ifndef __attribute__
 # if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 8)
 #  define __attribute__(x) /* empty */

daemon/inotify.c

@@ -56,13 +56,6 @@ optgroup_inotify_available (void)
 {
   return 1;
 }
-#else /* !HAVE_SYS_INOTIFY_H */
-int
-optgroup_inotify_available (void)
-{
-  return 0;
-}
-#endif
 
 /* Because inotify_init does NEED_ROOT, NEED_INOTIFY implies NEED_ROOT. */
 #define NEED_INOTIFY(errcode)						\
@@ -78,7 +71,6 @@ optgroup_inotify_available (void)
 int
 do_inotify_init (int max_events)
 {
-#ifdef HAVE_SYS_INOTIFY_H
   FILE *fp;
 
   NEED_ROOT (, return -1);
@@ -129,15 +121,11 @@ do_inotify_init (int max_events)
 #endif
 
   return 0;
-#else
-  NOT_AVAILABLE (-1);
-#endif
 }
 
 int
 do_inotify_close (void)
 {
-#ifdef HAVE_SYS_INOTIFY_H
   NEED_INOTIFY (-1);
 
   if (inotify_fd == -1) {
@@ -154,15 +142,11 @@ do_inotify_close (void)
   inotify_posn = 0;
 
   return 0;
-#else
-  NOT_AVAILABLE (-1);
-#endif
 }
 
 int64_t
 do_inotify_add_watch (const char *path, int mask)
 {
-#ifdef HAVE_SYS_INOTIFY_H
   int64_t r;
   char *buf;
 
@@ -182,15 +166,11 @@ do_inotify_add_watch (const char *path, int mask)
   }
 
   return r;
-#else
-  NOT_AVAILABLE (-1);
-#endif
 }
 
 int
 do_inotify_rm_watch (int wd)
 {
-#ifdef HAVE_SYS_INOTIFY_H
   NEED_INOTIFY (-1);
 
   if (inotify_rm_watch (inotify_fd, wd) == -1) {
@@ -199,15 +179,11 @@ do_inotify_rm_watch (int wd)
   }
 
   return 0;
-#else
-  NOT_AVAILABLE (-1);
-#endif
 }
 
 guestfs_int_inotify_event_list *
 do_inotify_read (void)
 {
-#ifdef HAVE_SYS_INOTIFY_H
   int space;
   guestfs_int_inotify_event_list *ret;
 
@@ -313,15 +289,11 @@ do_inotify_read (void)
   xdr_free ((xdrproc_t) xdr_guestfs_int_inotify_event_list, (char *) ret);
   free (ret);
   return NULL;
-#else
-  NOT_AVAILABLE (NULL);
-#endif
 }
 
 char **
 do_inotify_files (void)
 {
-#ifdef HAVE_SYS_INOTIFY_H
   char **ret = NULL;
   int size = 0, alloc = 0;
   unsigned int i;
@@ -404,7 +376,54 @@ do_inotify_files (void)
 
   unlink (tempfile);
   return NULL;
-#else
-  NOT_AVAILABLE (NULL);
-#endif
 }
+
+#else /* !HAVE_SYS_INOTIFY_H */
+
+/* Note that the wrapper code (daemon/stubs.c) ensures that the
+ * functions below are never called because optgroup_inotify_available
+ * returns false.
+ */
+int
+optgroup_inotify_available (void)
+{
+  return 0;
+}
+
+int
+do_inotify_init (int max_events)
+{
+  abort ();
+}
+
+int
+do_inotify_close (void)
+{
+  abort ();
+}
+
+int64_t
+do_inotify_add_watch (const char *path, int mask)
+{
+  abort ();
+}
+
+int
+do_inotify_rm_watch (int wd)
+{
+  abort ();
+}
+
+guestfs_int_inotify_event_list *
+do_inotify_read (void)
+{
+  abort ();
+}
+
+char **
+do_inotify_files (void)
+{
+  abort ();
+}
+
+#endif

daemon/mknod.c

@@ -37,18 +37,10 @@ optgroup_mknod_available (void)
 {
   return 1;
 }
-#else
-int
-optgroup_mknod_available (void)
-{
-  return 0;
-}
-#endif
 
 int
 do_mknod (int mode, int devmajor, int devminor, const char *path)
 {
-#ifdef HAVE_MKNOD
   int r;
 
   if (mode < 0) {
@@ -66,9 +58,6 @@ do_mknod (int mode, int devmajor, int devminor, const char *path)
   }
 
   return 0;
-#else
-  NOT_AVAILABLE (-1);
-#endif
 }
 
 int
@@ -88,3 +77,37 @@ do_mknod_c (int mode, int devmajor, int devminor, const char *path)
 {
   return do_mknod (mode | S_IFCHR, devmajor, devminor, path);
 }
+
+#else
+
+int
+optgroup_mknod_available (void)
+{
+  return 0;
+}
+
+int
+do_mknod (int mode, int devmajor, int devminor, const char *path)
+{
+  abort ();
+}
+
+int
+do_mkfifo (int mode, const char *path)
+{
+  abort ();
+}
+
+int
+do_mknod_b (int mode, int devmajor, int devminor, const char *path)
+{
+  abort ();
+}
+
+int
+do_mknod_c (int mode, int devmajor, int devminor, const char *path)
+{
+  abort ();
+}
+
+#endif

daemon/realpath.c

@@ -36,20 +36,17 @@
 #define NAME_MAX FILENAME_MAX
 #endif
 
+#ifdef HAVE_REALPATH
+
 int
 optgroup_realpath_available (void)
 {
-#ifdef HAVE_REALPATH
   return 1;
-#else
-  return 0;
-#endif
 }
 
 char *
 do_realpath (const char *path)
 {
-#ifdef HAVE_REALPATH
   char *ret;
 
   CHROOT_IN;
@@ -61,11 +58,24 @@ do_realpath (const char *path)
   }
 
   return ret;			/* caller frees */
-#else
-  NOT_AVAILABLE (NULL);
-#endif
 }
 
+#else /* !HAVE_REALPATH */
+
+int
+optgroup_realpath_available (void)
+{
+  return 0;
+}
+
+char *
+do_realpath (const char *path)
+{
+  abort ();
+}
+
+#endif /* !HAVE_REALPATH */
+
 static int find_path_element (int fd_cwd, char *name, size_t *name_len_ret);
 
 char *

daemon/selinux.c

@@ -32,18 +32,12 @@
 #include "optgroups.h"
 
 #if defined(HAVE_LIBSELINUX)
+
 int
 optgroup_selinux_available (void)
 {
   return 1;
 }
-#else /* !HAVE_LIBSELINUX */
-int
-optgroup_selinux_available (void)
-{
-  return 0;
-}
-#endif /* !HAVE_LIBSELINUX */
 
 /* setcon is only valid under the following circumstances:
  * - single threaded
@@ -52,7 +46,7 @@ optgroup_selinux_available (void)
 int
 do_setcon (const char *context)
 {
-#if defined(HAVE_LIBSELINUX) && defined(HAVE_SETCON)
+#if defined(HAVE_SETCON)
   if (setcon ((char *) context) == -1) {
     reply_with_perror ("setcon");
     return -1;
@@ -60,14 +54,15 @@ do_setcon (const char *context)
 
   return 0;
 #else
-  NOT_AVAILABLE (-1);
+  reply_with_error ("function not available");
+  return -1;
 #endif
 }
 
 char *
 do_getcon (void)
 {
-#if defined(HAVE_LIBSELINUX) && defined(HAVE_GETCON)
+#if defined(HAVE_GETCON)
   security_context_t context;
   char *r;
 
@@ -85,6 +80,29 @@ do_getcon (void)
 
   return r;                     /* caller frees */
 #else
-  NOT_AVAILABLE (NULL);
+  reply_with_error ("function not available");
+  return NULL;
 #endif
 }
+
+#else /* !HAVE_LIBSELINUX */
+
+int
+optgroup_selinux_available (void)
+{
+  return 0;
+}
+
+int
+do_setcon (const char *context)
+{
+  abort ();
+}
+
+char *
+do_getcon (void)
+{
+  abort ();
+}
+
+#endif /* !HAVE_LIBSELINUX */

daemon/xattr.c

@@ -511,6 +511,11 @@ do_lgetxattr (const char *path, const char *name, size_t *size_r)
 }
 
 #else /* no xattr.h */
+
+/* Note that the wrapper code (daemon/stubs.c) ensures that the
+ * functions below are never called because
+ * optgroup_linuxxattrs_available returns false.
+ */
 int
 optgroup_linuxxattrs_available (void)
 {
@@ -520,55 +525,55 @@ optgroup_linuxxattrs_available (void)
 guestfs_int_xattr_list *
 do_getxattrs (const char *path)
 {
-  NOT_AVAILABLE (NULL);
+  abort ();
 }
 
 guestfs_int_xattr_list *
 do_lgetxattrs (const char *path)
 {
-  NOT_AVAILABLE (NULL);
+  abort ();
 }
 
 int
 do_setxattr (const char *xattr, const char *val, int vallen, const char *path)
 {
-  NOT_AVAILABLE (-1);
+  abort ();
 }
 
 int
 do_lsetxattr (const char *xattr, const char *val, int vallen, const char *path)
 {
-  NOT_AVAILABLE (-1);
+  abort ();
 }
 
 int
 do_removexattr (const char *xattr, const char *path)
 {
-  NOT_AVAILABLE (-1);
+  abort ();
 }
 
 int
 do_lremovexattr (const char *xattr, const char *path)
 {
-  NOT_AVAILABLE (-1);
+  abort ();
 }
 
 guestfs_int_xattr_list *
 do_lxattrlist (const char *path, char *const *names)
 {
-  NOT_AVAILABLE (NULL);
+  abort ();
 }
 
 char *
 do_getxattr (const char *path, const char *name, size_t *size_r)
 {
-  NOT_AVAILABLE (NULL);
+  abort ();
 }
 
 char *
 do_lgetxattr (const char *path, const char *name, size_t *size_r)
 {
-  NOT_AVAILABLE (NULL);
+  abort ();
 }
 
 #endif /* no xattr.h */

generator/generator_daemon.ml

@@ -75,12 +75,14 @@ and generate_daemon_actions () =
   pr "#include \"c-ctype.h\"\n";
   pr "#include \"guestfs_protocol.h\"\n";
   pr "#include \"actions.h\"\n";
+  pr "#include \"optgroups.h\"\n";
   pr "\n";
 
   List.iter (
-    fun (name, (ret, args, optargs), _, _, _, _, _) ->
+    fun (name, (ret, args, optargs), _, flags, _, _, _) ->
       (* Generate server-side stubs. *)
-      pr "static void %s_stub (XDR *xdr_in)\n" name;
+      pr "static void\n";
+      pr "%s_stub (XDR *xdr_in)\n" name;
       pr "{\n";
       (match ret with
        | RErr | RInt _ -> pr "  int r;\n"
@@ -122,6 +124,24 @@ and generate_daemon_actions () =
       let is_filein =
         List.exists (function FileIn _ -> true | _ -> false) args in
 
+      (* Reject Optional functions that are not available (RHBZ#679737). *)
+      List.iter (
+        function
+        | Optional group ->
+          pr "  /* The caller should have checked before calling this. */\n";
+          pr "  if (! optgroup_%s_available ()) {\n" group;
+          if is_filein then
+            pr "    cancel_receive ();\n";
+          pr "    reply_with_error (\"feature '%%s' is not available in this\\n\"\n";
+          pr "                      \"build of libguestfs.  Read 'AVAILABILITY' in the guestfs(3) man page for\\n\"\n";
+          pr "                      \"how to check for the availability of features.\",\n";
+          pr "                      \"%s\");\n" group;
+          pr "    goto done;\n";
+          pr "  }\n";
+          pr "\n"
+        | _ -> ()
+      ) flags;
+
       (* Reject unknown optional arguments.
        * Note this code is included even for calls with no optional
        * args because the caller must not pass optargs_bitmask != 0